diff --git a/nx.json b/nx.json index 62a03429e..ca98aa141 100644 --- a/nx.json +++ b/nx.json @@ -47,6 +47,11 @@ "outputs": ["{projectRoot}/dist"], "cache": true }, + "package:js": { "dependsOn": ["build"] }, + "package:python": { "dependsOn": ["build"] }, + "package:java": { "dependsOn": ["build"] }, + "package:dotnet": { "dependsOn": ["build"] }, + "package:go": { "dependsOn": ["build"] }, "test": { "dependsOn": [ "build", diff --git a/packages/@cdktn/cli-core/src/lib/cdktf-stack.ts b/packages/@cdktn/cli-core/src/lib/cdktf-stack.ts index ce6c19511..2d85a8a0d 100644 --- a/packages/@cdktn/cli-core/src/lib/cdktf-stack.ts +++ b/packages/@cdktn/cli-core/src/lib/cdktf-stack.ts @@ -17,9 +17,12 @@ import { tryReadGeneratedConfigurationFile, tryRemoveGeneratedConfigurationFile, } from "./models/terraform-cli"; -import { ProviderConstraint } from "./dependencies/dependency-manager"; import { terraformJsonSchema, TerraformStack } from "./terraform-json"; -import { TerraformProviderLock } from "./terraform-provider-lock"; +import { + TerraformProviderLock, + needsLockfileUpdate, + needsUpgrade, +} from "./terraform-provider-lock"; import { convertConfigurationFile } from "./convert"; export type StackUpdate = @@ -299,15 +302,7 @@ export class CdktfStack { private requiredProviders() { // Read required providers from the stack output const requiredProviders = this.parsedContent.terraform?.required_providers; - - return Object.values(requiredProviders || {}).reduce( - (acc, obj) => { - const constraint = new ProviderConstraint(obj.source, obj.version); - acc[constraint.source] = constraint; - return acc; - }, - {} as Record, - ); + return Object.values(requiredProviders || {}); } private async checkNeedsLockfileUpdate(): Promise { @@ -323,46 +318,12 @@ export class CdktfStack { return true; } - const requiredProviders = this.requiredProviders(); - - for (const provider of Object.values(requiredProviders)) { - const hasProvider = await lock.hasMatchingProvider(provider); - if (!hasProvider) { - // If we don't have a provider or version doesn't match, we need to init - return true; - } - } - - return false; + return needsLockfileUpdate(this.requiredProviders(), lock); } private async checkNeedsUpgrade(): Promise { const lock = new TerraformProviderLock(this.stack.workingDirectory); - const allProviders = this.requiredProviders(); - const lockedProviders = Object.values(await lock.providers()); - - // Check if any provider contained in `providers` violates constraints in `lockedProviders` - // Upgrade if some provider constraint not met - // If a provider wasn't preset in lockedProviders, that's fine; it will just get added - return lockedProviders.some((lockedProvider) => { - const lockedConstraint = lockedProvider.constraints; - if (!lockedConstraint) { - // Provider lock doesn't have a constraint specified, so we can't check. - // This shouldn't happen - logger.debug( - `Provider lock doesn't have a constraint for ${lockedProvider.name}`, - ); - return false; - } - - const provider = allProviders[lockedConstraint.source]; - if (!provider) { - // else no longer using this provider, so won't cause problems - return; - } - - return !lockedConstraint.matchesVersion(provider.version ?? ">0"); - }); + return needsUpgrade(this.requiredProviders(), lock); } private async run(cb: () => Promise) { diff --git a/packages/@cdktn/cli-core/src/lib/dependencies/cdktf-config-manager.ts b/packages/@cdktn/cli-core/src/lib/dependencies/cdktf-config-manager.ts index b49c6b72b..f2554cbf6 100644 --- a/packages/@cdktn/cli-core/src/lib/dependencies/cdktf-config-manager.ts +++ b/packages/@cdktn/cli-core/src/lib/dependencies/cdktf-config-manager.ts @@ -1,37 +1,62 @@ // Copyright (c) HashiCorp, Inc // SPDX-License-Identifier: MPL-2.0 +import { + Registry, + TerraformDependencyConstraint, + registryForTargetVersions, +} from "@cdktn/commons"; import { CdktfConfig } from "../cdktf-config"; import { ProviderConstraint } from "./dependency-manager"; +/** + * Drops the host when it is the project's own registry, so cdktf.json keeps the + * short form. A source qualified with any other host stays qualified. + */ +function simplifySource(source: string, registry: Registry): string { + const prefix = `${registry.hostname}/`; + return source.startsWith(prefix) ? source.slice(prefix.length) : source; +} + /** * facilitates adding provider dependencies to cdktf.json */ export class CdktfConfigManager { private config: CdktfConfig = CdktfConfig.read(); + /** Registry an unqualified entry in this cdktf.json resolves against. */ + private get registry(): Registry { + return registryForTargetVersions(this.config.targetVersions); + } + public async hasProvider(constraint: ProviderConstraint): Promise { + const registry = this.registry; return this.config.terraformProviders.some( (provider) => - ProviderConstraint.fromConfigEntry(provider).source === + ProviderConstraint.fromConfigEntry(provider, registry).source === constraint.source, ); } private async setProvider(constraint: ProviderConstraint): Promise { + const registry = this.registry; + const isSame = (provider: string | TerraformDependencyConstraint) => + ProviderConstraint.fromConfigEntry(provider, registry).source === + constraint.source; + + // An update keeps the entry's own spelling, so a host the user wrote stays. + const existing = this.config.terraformProviders.find(isSame); + const source = + typeof existing === "string" + ? existing.split("@")[0].trim() + : simplifySource(constraint.source, registry); + // excluding the constraint to be added (if it already existed) const currentProviders = this.config.terraformProviders.filter( - (provider) => - ProviderConstraint.fromConfigEntry(provider).source !== - constraint.source, - ); - - const simplifiedSource = constraint.source.replace( - "registry.terraform.io/", - "", + (provider) => !isSame(provider), ); const provider = - simplifiedSource + (constraint.version ? `@${constraint.version}` : ""); + source + (constraint.version ? `@${constraint.version}` : ""); currentProviders.push(provider); this.config.writeTerraformProviders(currentProviders); diff --git a/packages/@cdktn/cli-core/src/lib/dependencies/dependency-manager.ts b/packages/@cdktn/cli-core/src/lib/dependencies/dependency-manager.ts index 05b7da078..433dde8b5 100644 --- a/packages/@cdktn/cli-core/src/lib/dependencies/dependency-manager.ts +++ b/packages/@cdktn/cli-core/src/lib/dependencies/dependency-manager.ts @@ -58,12 +58,24 @@ export class ProviderConstraint { // TODO: add examples to cli command description (i.e. =,~>.> etc.) // if no version constraint is specified, we assume the latest version // if specific version is specified without e.g. =, we allow patch level increments (e.g. ~>2.12 for "2.12") + /** Whether the source as given named a host, rather than gaining one here. */ + private readonly namedHost: boolean; + constructor( source: string, public readonly version: string | undefined, registry: Registry = TERRAFORM_REGISTRY, ) { this.source = normalizeProviderSource(source, registry); + this.namedHost = source.split("/").length === 3; + } + + /** + * Source for a `required_providers` block. Keeps a host the user wrote; drops + * one added by normalization, since a host pins the CLI to that registry. + */ + public get requiredProvidersSource(): string { + return this.namedHost ? this.source : `${this.namespace}/${this.name}`; } static fromConfigEntry( diff --git a/packages/@cdktn/cli-core/src/lib/local-provider-versions.ts b/packages/@cdktn/cli-core/src/lib/local-provider-versions.ts index 792f21248..d7048f0ec 100644 --- a/packages/@cdktn/cli-core/src/lib/local-provider-versions.ts +++ b/packages/@cdktn/cli-core/src/lib/local-provider-versions.ts @@ -1,13 +1,10 @@ // Copyright (c) HashiCorp, Inc // SPDX-License-Identifier: MPL-2.0 -import { Errors, logger } from "@cdktn/commons"; +import { Errors, logger, registryForHostname } from "@cdktn/commons"; import fs from "fs-extra"; import path from "path"; import { CdktfConfig } from "./cdktf-config"; -import { - DEFAULT_HOSTNAME, - DEFAULT_NAMESPACE, -} from "./dependencies/dependency-manager"; +import { DEFAULT_NAMESPACE } from "./dependencies/dependency-manager"; // TODO: move this to @cdktn/commons // tracked here https://github.com/hashicorp/terraform-cdk/issues/1814 @@ -46,11 +43,15 @@ export class LocalProviderVersions { this.versions = Object.fromEntries( Object.entries(providerVersions).map(([providerFqn, versions]) => { + // Any public registry host, not just Terraform's: the host recorded here + // is whichever registry the fetching CLI used. + const [maybeHost, ...rest] = providerFqn.split("/"); + const withoutHost = registryForHostname(maybeHost) + ? rest.join("/") + : providerFqn; + return [ - providerFqn - .replace(`${DEFAULT_HOSTNAME}/`, "") - .replace(`${DEFAULT_NAMESPACE}/`, "") - .toLowerCase(), + withoutHost.replace(`${DEFAULT_NAMESPACE}/`, "").toLowerCase(), versions, ]; }), diff --git a/packages/@cdktn/cli-core/src/lib/terraform-provider-lock.ts b/packages/@cdktn/cli-core/src/lib/terraform-provider-lock.ts index 079d59245..b56dd1fe2 100644 --- a/packages/@cdktn/cli-core/src/lib/terraform-provider-lock.ts +++ b/packages/@cdktn/cli-core/src/lib/terraform-provider-lock.ts @@ -6,7 +6,7 @@ import path from "path"; import fs from "fs/promises"; import { ProviderConstraint } from "./dependencies/dependency-manager"; -import { logger } from "@cdktn/commons"; +import { PUBLIC_REGISTRIES, logger } from "@cdktn/commons"; const TerraformLockFileName = ".terraform.lock.hcl"; @@ -22,6 +22,73 @@ type TerraformProviderLockFileData = { }; }; +type RequiredProvider = { source: string; version?: string }; + +// A source that names a host is locked under exactly that host. A bare one is +// locked under whichever public registry the CLI resolved it from, so either +// counts: OpenTofu migrates a Terraform entry itself on init. +function lockAddressesFor( + source: string, + version?: string, +): ProviderConstraint[] { + if (source.split("/").length === 3) { + return [new ProviderConstraint(source, version)]; + } + return PUBLIC_REGISTRIES.map( + (registry) => new ProviderConstraint(source, version, registry), + ); +} + +/** Whether `init` should be followed by a `providers lock` refresh. */ +export async function needsLockfileUpdate( + required: RequiredProvider[], + lock: TerraformProviderLock, +): Promise { + for (const { source, version } of required) { + let locked = false; + for (const address of lockAddressesFor(source, version)) { + if (await lock.hasMatchingProvider(address)) { + locked = true; + break; + } + } + if (!locked) { + return true; + } + } + return false; +} + +/** Whether a locked version no longer satisfies a requirement, needing `init -upgrade`. */ +export async function needsUpgrade( + required: RequiredProvider[], + lock: TerraformProviderLock, +): Promise { + const lockedProviders = Object.values(await lock.providers()); + return lockedProviders.some((lockedProvider) => { + const lockedConstraint = lockedProvider.constraints; + if (!lockedConstraint) { + // Provider lock doesn't have a constraint specified, so we can't check. + logger.debug( + `Provider lock doesn't have a constraint for ${lockedProvider.name}`, + ); + return false; + } + + const provider = required.find(({ source, version }) => + lockAddressesFor(source, version).some( + (address) => address.source === lockedConstraint.source, + ), + ); + // No longer required by this stack, so it can't block the upgrade. + if (!provider) { + return false; + } + + return !lockedConstraint.matchesVersion(provider.version ?? ">0"); + }); +} + export class TerraformProviderLock { private _providerLockData: TerraformProviderLockFileData | null; constructor(private stackWorkingDirectory: string) { diff --git a/packages/@cdktn/cli-core/src/test/lib/dependencies/cdktf-config-manager.test.ts b/packages/@cdktn/cli-core/src/test/lib/dependencies/cdktf-config-manager.test.ts new file mode 100644 index 000000000..e67f69568 --- /dev/null +++ b/packages/@cdktn/cli-core/src/test/lib/dependencies/cdktf-config-manager.test.ts @@ -0,0 +1,131 @@ +// Copyright (c) HashiCorp, Inc +// SPDX-License-Identifier: MPL-2.0 +import * as fs from "fs"; +import * as os from "os"; +import * as path from "path"; +import { + OPENTOFU_REGISTRY, + TERRAFORM_REGISTRY, + TerraformTargetVersions, +} from "@cdktn/commons"; +import { CdktfConfigManager } from "../../../lib/dependencies/cdktf-config-manager"; +import { ProviderConstraint } from "../../../lib/dependencies/dependency-manager"; + +describe("CdktfConfigManager", () => { + let projectDir: string; + let originalCwd: string; + + const writeConfig = ( + providers: string[], + targetVersions?: TerraformTargetVersions, + ) => + fs.writeFileSync( + path.join(projectDir, "cdktf.json"), + JSON.stringify({ + language: "typescript", + app: "npx tsx main.ts", + terraformProviders: providers, + ...(targetVersions ? { targetVersions } : {}), + }), + ); + + const readProviders = (): string[] => + JSON.parse(fs.readFileSync(path.join(projectDir, "cdktf.json"), "utf8")) + .terraformProviders; + + beforeEach(() => { + originalCwd = process.cwd(); + projectDir = fs.mkdtempSync(path.join(os.tmpdir(), "cdktf-config-mgr-")); + process.chdir(projectDir); + }); + + afterEach(() => { + process.chdir(originalCwd); + }); + + describe("a Terraform project", () => { + it("writes the short form and replaces the existing entry", async () => { + writeConfig(["hashicorp/random@=3.1.3"]); + + await new CdktfConfigManager().updateProvider( + new ProviderConstraint("random", "=3.2.0", TERRAFORM_REGISTRY), + ); + + expect(readProviders()).toEqual(["hashicorp/random@=3.2.0"]); + }); + }); + + describe("an OpenTofu project", () => { + const opentofu: TerraformTargetVersions = { opentofu: ">=1.6.0" }; + + it("writes the short form rather than qualifying with the tofu registry", async () => { + writeConfig(["hashicorp/random@=3.1.3"], opentofu); + + await new CdktfConfigManager().updateProvider( + new ProviderConstraint("random", "=3.2.0", OPENTOFU_REGISTRY), + ); + + expect(readProviders()).toEqual(["hashicorp/random@=3.2.0"]); + }); + + it.each([ + ["a shorthand", "random"], + ["a qualified", "registry.opentofu.org/hashicorp/random"], + ])( + "keeps the host of an explicitly qualified entry on %s upgrade", + async (_name, upgradeSource) => { + writeConfig( + ["registry.opentofu.org/hashicorp/random@=3.6.0"], + opentofu, + ); + + await new CdktfConfigManager().updateProvider( + new ProviderConstraint(upgradeSource, "=3.6.1", OPENTOFU_REGISTRY), + ); + + expect(readProviders()).toEqual([ + "registry.opentofu.org/hashicorp/random@=3.6.1", + ]); + }, + ); + + it("recognises an unqualified entry as the same provider", async () => { + writeConfig(["hashicorp/random@=3.1.3"], opentofu); + + await expect( + new CdktfConfigManager().hasProvider( + new ProviderConstraint("random", "=3.1.3", OPENTOFU_REGISTRY), + ), + ).resolves.toBe(true); + }); + + it("keeps a source explicitly qualified with another registry", async () => { + writeConfig([], opentofu); + + await new CdktfConfigManager().addProvider( + new ProviderConstraint( + "registry.terraform.io/hashicorp/random", + "=3.2.0", + OPENTOFU_REGISTRY, + ), + ); + + expect(readProviders()).toEqual([ + "registry.terraform.io/hashicorp/random@=3.2.0", + ]); + }); + + it("treats an explicitly qualified entry as distinct from the project's own", async () => { + writeConfig(["registry.terraform.io/hashicorp/random@=3.1.3"], opentofu); + + await new CdktfConfigManager().addProvider( + new ProviderConstraint("random", "=3.2.0", OPENTOFU_REGISTRY), + ); + + expect(readProviders()).toEqual([ + "registry.terraform.io/hashicorp/random@=3.1.3", + "hashicorp/random@=3.2.0", + ]); + }); + }); +}); diff --git a/packages/@cdktn/cli-core/src/test/lib/dependencies/dependency-manager.test.ts b/packages/@cdktn/cli-core/src/test/lib/dependencies/dependency-manager.test.ts index 40b6c60bf..c44e31824 100644 --- a/packages/@cdktn/cli-core/src/test/lib/dependencies/dependency-manager.test.ts +++ b/packages/@cdktn/cli-core/src/test/lib/dependencies/dependency-manager.test.ts @@ -1,6 +1,6 @@ // Copyright (c) HashiCorp, Inc // SPDX-License-Identifier: MPL-2.0 -import { Language } from "@cdktn/commons"; +import { Language, OPENTOFU_REGISTRY } from "@cdktn/commons"; import { ProviderConstraint, DependencyManager, @@ -8,6 +8,42 @@ import { describe("dependency manager", () => { describe("ProviderConstraint", () => { + describe("requiredProvidersSource", () => { + it("drops a host that normalization added", () => { + expect( + new ProviderConstraint("random", "=3.6.1", OPENTOFU_REGISTRY) + .requiredProvidersSource, + ).toBe("hashicorp/random"); + expect( + new ProviderConstraint( + "hashicorp/random", + "=3.6.1", + OPENTOFU_REGISTRY, + ).requiredProvidersSource, + ).toBe("hashicorp/random"); + }); + + it("keeps a public host the user wrote", () => { + expect( + new ProviderConstraint( + "registry.terraform.io/hashicorp/random", + "=3.6.1", + OPENTOFU_REGISTRY, + ).requiredProvidersSource, + ).toBe("registry.terraform.io/hashicorp/random"); + }); + + it("keeps a private host", () => { + expect( + new ProviderConstraint( + "registry.example.com/acme/thing", + "1.0.0", + OPENTOFU_REGISTRY, + ).requiredProvidersSource, + ).toBe("registry.example.com/acme/thing"); + }); + }); + it.each(["aws", "hashicorp/aws", "registry.terraform.io/hashicorp/aws"])( "should parse a simple constraint from '%s'", (configEntry) => { diff --git a/packages/@cdktn/cli-core/src/test/lib/local-provider-versions.test.ts b/packages/@cdktn/cli-core/src/test/lib/local-provider-versions.test.ts new file mode 100644 index 000000000..27eb1be9f --- /dev/null +++ b/packages/@cdktn/cli-core/src/test/lib/local-provider-versions.test.ts @@ -0,0 +1,71 @@ +// Copyright (c) HashiCorp, Inc +// SPDX-License-Identifier: MPL-2.0 +import * as fs from "fs"; +import * as os from "os"; +import * as path from "path"; +import { LocalProviderVersions } from "../../lib/local-provider-versions"; + +describe("LocalProviderVersions", () => { + let projectDir: string; + let originalCwd: string; + + const writeProject = (versions: Record) => { + fs.writeFileSync( + path.join(projectDir, "cdktf.json"), + JSON.stringify({ language: "typescript", app: "npx tsx main.ts" }), + ); + fs.mkdirSync(path.join(projectDir, ".gen"), { recursive: true }); + fs.writeFileSync( + path.join(projectDir, ".gen", "versions.json"), + JSON.stringify(versions), + ); + }; + + beforeEach(() => { + originalCwd = process.cwd(); + projectDir = fs.mkdtempSync(path.join(os.tmpdir(), "local-versions-")); + process.chdir(projectDir); + }); + + afterEach(() => { + process.chdir(originalCwd); + }); + + it("reads a provider fetched by Terraform", async () => { + writeProject({ "registry.terraform.io/hashicorp/random": "3.2.0" }); + + await expect( + new LocalProviderVersions().versionForProvider("random"), + ).resolves.toBe("3.2.0"); + }); + + it("reads a provider fetched by OpenTofu", async () => { + writeProject({ "registry.opentofu.org/hashicorp/random": "3.2.0" }); + + await expect( + new LocalProviderVersions().versionForProvider("random"), + ).resolves.toBe("3.2.0"); + }); + + it("keeps a private registry host in the key", async () => { + writeProject({ "my.registry.example.com/hashicorp/random": "3.2.0" }); + + const versions = new LocalProviderVersions(); + await expect( + versions.versionForProvider("random"), + ).resolves.toBeUndefined(); + // The default namespace is still dropped wherever it appears, which is + // pre-existing behaviour this change does not alter. + await expect( + versions.versionForProvider("my.registry.example.com/random"), + ).resolves.toBe("3.2.0"); + }); + + it("keeps a non-default namespace", async () => { + writeProject({ "registry.opentofu.org/kreuzwerker/docker": "3.9.0" }); + + await expect( + new LocalProviderVersions().versionForProvider("kreuzwerker/docker"), + ).resolves.toBe("3.9.0"); + }); +}); diff --git a/packages/@cdktn/cli-core/src/test/lib/terraform-provider-lock.test.ts b/packages/@cdktn/cli-core/src/test/lib/terraform-provider-lock.test.ts index 78a38efe5..40f38d489 100644 --- a/packages/@cdktn/cli-core/src/test/lib/terraform-provider-lock.test.ts +++ b/packages/@cdktn/cli-core/src/test/lib/terraform-provider-lock.test.ts @@ -3,7 +3,11 @@ * SPDX-License-Identifier: MPL-2.0 */ -import { TerraformProviderLock } from "../../lib/terraform-provider-lock"; +import { + TerraformProviderLock, + needsLockfileUpdate, + needsUpgrade, +} from "../../lib/terraform-provider-lock"; import { readFile, stat } from "fs/promises"; import * as path from "path"; import { ProviderConstraint } from "../../lib/dependencies/dependency-manager"; @@ -203,3 +207,85 @@ describe("TerraformProviderLock", () => { expect(stat).toHaveBeenCalledWith(path.join("test", ".terraform.lock.hcl")); }); }); + +describe("lock decisions", () => { + const terraformEntry = "registry.terraform.io/hashicorp/test"; + const opentofuEntry = "registry.opentofu.org/hashicorp/test"; + const bare = [{ source: "hashicorp/test", version: "1.2" }]; + + const lockWith = ( + entries: { name: string; version: string; constraints: string }[], + ) => { + (readFile as jest.Mock).mockResolvedValue( + generateProviderLockFileContents(entries), + ); + return new TerraformProviderLock("test"); + }; + + beforeEach(() => { + jest.clearAllMocks(); + }); + + describe("needsLockfileUpdate", () => { + it.each([ + ["Terraform", terraformEntry], + ["OpenTofu", opentofuEntry], + ])("accepts a bare source locked by %s", async (_cli, name) => { + const lock = lockWith([{ name, version: "1.2.3", constraints: "1.2" }]); + await expect(needsLockfileUpdate(bare, lock)).resolves.toBe(false); + }); + + it("refreshes when the provider is not locked at all", async () => { + const lock = lockWith([]); + await expect(needsLockfileUpdate(bare, lock)).resolves.toBe(true); + }); + + it("does not let a different public registry satisfy an explicit host", async () => { + const lock = lockWith([ + { name: opentofuEntry, version: "1.2.3", constraints: "1.2" }, + ]); + await expect( + needsLockfileUpdate([{ source: terraformEntry, version: "1.2" }], lock), + ).resolves.toBe(true); + }); + }); + + describe("needsUpgrade", () => { + // A project deployed with Terraform and then bumped under OpenTofu: without + // `-upgrade`, OpenTofu rejects the plain init. + it("upgrades a bump locked under the other CLI's registry", async () => { + const lock = lockWith([ + { name: terraformEntry, version: "1.0.0", constraints: "1.0.0" }, + ]); + await expect(needsUpgrade(bare, lock)).resolves.toBe(true); + }); + + it("does not upgrade when the locked version still satisfies the requirement", async () => { + const lock = lockWith([ + { name: opentofuEntry, version: "1.2.3", constraints: "1.2" }, + ]); + await expect(needsUpgrade(bare, lock)).resolves.toBe(false); + }); + + // After a switch the lock can hold both; a stale entry costs one extra + // `-upgrade`, which is safe. + it("upgrades when either registry's entry conflicts", async () => { + const lock = lockWith([ + { name: terraformEntry, version: "1.0.0", constraints: "1.0.0" }, + { name: opentofuEntry, version: "1.2.3", constraints: "1.2" }, + ]); + await expect(needsUpgrade(bare, lock)).resolves.toBe(true); + }); + + it("ignores a locked provider the stack no longer requires", async () => { + const lock = lockWith([ + { + name: "registry.terraform.io/hashicorp/other", + version: "1.0.0", + constraints: "1.0.0", + }, + ]); + await expect(needsUpgrade(bare, lock)).resolves.toBe(false); + }); + }); +}); diff --git a/packages/@cdktn/provider-generator/src/get/constructs-maker.ts b/packages/@cdktn/provider-generator/src/get/constructs-maker.ts index 28b295732..ac9b8766e 100644 --- a/packages/@cdktn/provider-generator/src/get/constructs-maker.ts +++ b/packages/@cdktn/provider-generator/src/get/constructs-maker.ts @@ -529,7 +529,21 @@ export class ConstructsMaker { ); } + // Keys are fully qualified and the host is whichever registry the fetching CLI + // resolved from, so a provider regenerated this run can arrive under a different + // host than the file already holds. Carrying the old key forward as well would + // leave two entries for one provider, and the stale one sorts first. + const regenerated = new Set( + Object.keys(this.versions).map((fqpn) => + fqpn.split("/").slice(1).join("/"), + ), + ); + const versions = allowedConstraints.reduce((acc, constraint) => { + if (regenerated.has(constraint.fqn)) { + return acc; + } + const provider = Object.entries(previousVersions).find(([name]) => // This could be more refined, but it's good enough for now name.endsWith(constraint.fqn), diff --git a/packages/cdktn-cli/src/bin/cmds/handlers.ts b/packages/cdktn-cli/src/bin/cmds/handlers.ts index 293e59d39..1d9cae6ce 100644 --- a/packages/cdktn-cli/src/bin/cmds/handlers.ts +++ b/packages/cdktn-cli/src/bin/cmds/handlers.ts @@ -21,6 +21,7 @@ import { getPackageVersion, TerraformDependencyConstraint, ConstructsMakerProviderTarget, + registryForTargetVersions, } from "@cdktn/commons"; import { checkForEmptyDirectory, runInit } from "./helper/init"; @@ -723,8 +724,13 @@ export async function providerUpgrade(argv: any) { const constraintsToUpdate: ProviderConstraint[] = []; + // Resolve an unqualified argument the way this project's cdktf.json entries + // resolve. Defaulting to Terraform made `upgrade` normalize to a different + // host than the config, so an installed provider looked missing. + const registry = registryForTargetVersions(config.targetVersions); + for (const provider of argv.provider) { - const constraint = ProviderConstraint.fromConfigEntry(provider); + const constraint = ProviderConstraint.fromConfigEntry(provider, registry); const { addedLocalProvider } = await manager.upgradeProvider(constraint); if (addedLocalProvider) { constraintsToUpdate.push(constraint); @@ -760,7 +766,12 @@ export async function providerUpgrade(argv: any) { constraints, cleanDirectory: false, constraintsToGenerate: constraintsToUpdate.map( - (c) => new TerraformProviderConstraint(c), + (c) => + new TerraformProviderConstraint( + c.version + ? `${c.requiredProvidersSource}@${c.version}` + : c.requiredProvidersSource, + ), ), }); } diff --git a/test/Readme.md b/test/Readme.md index 66f077269..7683af6ad 100644 --- a/test/Readme.md +++ b/test/Readme.md @@ -77,14 +77,25 @@ Which tests execute the binary: `.terraform.versions.json` at the repository root drives the matrix: -- `available` — versions baked into the CI Docker image (see `Dockerfile`); - each is installed as `/usr/local/bin/terraform` with the `default` - version symlinked to `terraform`. +- `available` — Terraform versions baked into the CI Docker image (see + `Dockerfile`); each is installed as `/usr/local/bin/terraform` with + the `default` version symlinked to `terraform`. - `tested` — the subset run in CI workflows, exported as `TERRAFORM_BINARY_NAME=terraform` per matrix job. - -OpenTofu is currently **not** part of the CI image; OpenTofu runs are local -only for now. +- `opentofu.available` / `opentofu.default` — the same for OpenTofu, installed + as `tofu` with the default symlinked to `tofu`. + +OpenTofu does not run across the whole integration suite. A test opts in +through `pinnedRuntimes` in `tools/build-test-matrix.mjs`, which replaces the +`tested` cross-product for that test with an explicit list of +`{ product, version }` runtimes; `terraformOnly` marks tests that can never run +under OpenTofu. Unit tests in packages tagged `unit-test:terraform` also run +once under OpenTofu (`pr-unit.yml`). + +Running under OpenTofu exercises the CLI, not a project that _targets_ +OpenTofu. Code that resolves registries from `targetVersions` only takes its +OpenTofu path when the project declares it, so cover that with a fixture that +sets `targetVersions` — see `typescript/opentofu-target`. ### Local runs diff --git a/test/go/provider-add-command/test.ts b/test/go/provider-add-command/test.ts index bcc9c90a5..965f22d29 100644 --- a/test/go/provider-add-command/test.ts +++ b/test/go/provider-add-command/test.ts @@ -3,6 +3,7 @@ import { onPosix, onWindows, + providerVersion, sanitizeTimestamps, TestDriver, } from "../../test-helper"; @@ -42,9 +43,9 @@ describe("provider add command", () => { // const genVersionsFile = JSON.parse( // driver.readLocalFile("generated/versions.json") // ); - // expect( - // genVersionsFile["registry.terraform.io/hashicorp/local"] - // ).toEqual("2.2.3"); + // expect(providerVersion(genVersionsFile, "hashicorp/local")).toEqual( + // "2.2.3", + // ); }, 240_000, ); @@ -73,9 +74,9 @@ describe("provider add command", () => { const genVersionsFile = JSON.parse( driver.readLocalFile("generated/versions.json"), ); - expect( - genVersionsFile["registry.terraform.io/hashicorp/local"], - ).toEqual("2.2.3"); + expect(providerVersion(genVersionsFile, "hashicorp/local")).toEqual( + "2.2.3", + ); }, 120_000, ); diff --git a/test/run-against-dist.mjs b/test/run-against-dist.mjs index fbacf162f..4305d867e 100755 --- a/test/run-against-dist.mjs +++ b/test/run-against-dist.mjs @@ -190,14 +190,30 @@ try { stdio: ["ignore", "inherit", "inherit"], }); - // There's only one version in our local registry, so we don't need to specify - // a version here. Retry to ride out transient ECONNRESETs from Verdaccio's - // upstream fetch. + // Install the tarball itself rather than `cdktn-cli` by name: local builds all + // carry the same version, so pnpm can resolve an earlier build it already + // holds for it and silently test stale code (#488). The digit keeps + // cdktn-cli-core-*.tgz out of the match. + const cliTarballs = []; + const cliGlob = join(cdktnDist, "js", "cdktn-cli-[0-9]*.tgz").replaceAll( + "\\", + "/", + ); + for await (const tgz of glob(cliGlob)) { + cliTarballs.push(tgz); + } + if (cliTarballs.length !== 1) { + await fail( + `Expected exactly one cdktn-cli tarball in dist/js, found ${cliTarballs.length}: ${cliTarballs.join(", ")}`, + ); + } + + // Retry to ride out transient ECONNRESETs from Verdaccio's upstream fetch. for (let attempt = 1; attempt <= 3; attempt++) { try { await execa( packageManagerName, - ["add", "cdktn-cli", `--registry=${REGISTRY_URL}`], + ["add", cliTarballs[0], `--registry=${REGISTRY_URL}`], { cwd: staging, stdio: ["ignore", "inherit", "inherit"] }, ); break; diff --git a/test/test-helper.ts b/test/test-helper.ts index e10787907..6f6fd751f 100644 --- a/test/test-helper.ts +++ b/test/test-helper.ts @@ -48,6 +48,32 @@ export function packageJsonWithDependency(name: string, version?: string) { }); } +/** + * Looks up a provider version in a generated `versions.json`. Keys are fully qualified and carry the host of whichever + * registry the fetching CLI used, so match on the provider part instead of pinning one registry. + * + * @param versionsFile parsed contents of a generated `versions.json` + * @param fqn provider name as `namespace/name` + */ +export function providerVersion( + versionsFile: Record, + fqn: string, +): string | undefined { + const matches = Object.entries(versionsFile).filter( + ([key]) => key.split("/").slice(1).join("/") === fqn, + ); + // More than one means one entry per registry host for the same provider (#483). + // Returning either would let a stale version pass silently. + if (matches.length > 1) { + throw new Error( + `versions.json has ${matches.length} entries for ${fqn}: ${matches + .map(([key, version]) => `${key}=${version}`) + .join(", ")}`, + ); + } + return matches[0]?.[1]; +} + export class QueryableStack { private readonly stack: Record; constructor(stackInput: string) { @@ -366,10 +392,10 @@ export class TestDriver { }; /** - * runs terraform init and terraform validate in the output directory for the given stack name + * runs init and validate with the configured CLI in the output directory for the given stack name * @param stack the name of the stack to validate * @param baseDirectory an optional base directory for the cdktn project - * @returns the stdout of terraform validate + * @returns the stdout of validate */ async validate(stack: string, baseDirectory?: string) { const cwd = path.join( @@ -378,8 +404,10 @@ export class TestDriver { "stacks", stack, ); - await this.exec("terraform", ["init"], cwd); - const res = await this.exec("terraform", ["validate"], cwd); + // The image's bare `terraform` is the default version, so honour the CLI the matrix selected. + const binary = this.env.TERRAFORM_BINARY_NAME || "terraform"; + await this.exec(binary, ["init"], cwd); + const res = await this.exec(binary, ["validate"], cwd); return res.stdout; } diff --git a/test/typescript/opentofu-target/cdktf.json b/test/typescript/opentofu-target/cdktf.json new file mode 100644 index 000000000..dac27d013 --- /dev/null +++ b/test/typescript/opentofu-target/cdktf.json @@ -0,0 +1,12 @@ +{ + "language": "typescript", + "app": "npx tsx main.ts", + "terraformProviders": [ + "hashicorp/random@=3.6.0" + ], + "terraformModules": [], + "targetVersions": { + "opentofu": ">=1.6.0" + }, + "context": {} +} diff --git a/test/typescript/opentofu-target/main.ts b/test/typescript/opentofu-target/main.ts new file mode 100644 index 000000000..78d658aa5 --- /dev/null +++ b/test/typescript/opentofu-target/main.ts @@ -0,0 +1,19 @@ +// Copyright (c) HashiCorp, Inc +// SPDX-License-Identifier: MPL-2.0 +import { Construct } from "constructs"; +import { App, TerraformOutput, TerraformStack, Testing } from "cdktn"; +import { RandomProvider } from "./.gen/providers/random/provider"; +import { Id } from "./.gen/providers/random/id"; + +export class OpenTofuTarget extends TerraformStack { + constructor(scope: Construct, name: string) { + super(scope, name); + new RandomProvider(this, "random", {}); + const randomId = new Id(this, "id", { byteLength: 4 }); + new TerraformOutput(this, "id_hex", { value: randomId.hex }); + } +} + +const app = Testing.stubVersion(new App({ stackTraces: false })); +new OpenTofuTarget(app, "opentofu-target"); +app.synth(); diff --git a/test/typescript/opentofu-target/test.ts b/test/typescript/opentofu-target/test.ts new file mode 100644 index 000000000..dc9852eb3 --- /dev/null +++ b/test/typescript/opentofu-target/test.ts @@ -0,0 +1,84 @@ +// Copyright (c) HashiCorp, Inc +// SPDX-License-Identifier: MPL-2.0 +import * as fs from "fs"; +import * as path from "path"; +import { TestDriver, providerVersion } from "../../test-helper"; + +// The project declares an OpenTofu-only target. The matrix's other OpenTofu rows +// change only the CLI, so code that resolves registries from `targetVersions` is +// covered nowhere else (#494). +describe("project targeting OpenTofu", () => { + const stack = "opentofu-target"; + let driver: TestDriver; + + const lockFile = () => + fs.readFileSync( + path.join(driver.stackDirectory(stack), ".terraform.lock.hcl"), + "utf8", + ); + + beforeAll(async () => { + driver = new TestDriver(__dirname); + await driver.setupTypescriptProject(); + }, 500_000); + + test("generates bindings from the OpenTofu registry", () => { + const versions = JSON.parse(driver.readLocalFile(".gen/versions.json")); + expect(versions).toEqual({ + "registry.opentofu.org/hashicorp/random": "3.6.0", + }); + }); + + test("links generated docs to the OpenTofu registry", () => { + const provider = driver.readLocalFile( + ".gen/providers/random/provider/index.ts", + ); + expect(provider).toContain( + "https://search.opentofu.org/provider/hashicorp/random/v3.6.0/docs", + ); + expect(provider).not.toContain("registry.terraform.io/providers"); + }); + + // A host here would pin the CLI to that registry. + test("synthesizes the provider source without a registry host", async () => { + await driver.synth(); + const synthesized = JSON.parse(driver.synthesizedStackContentsRaw(stack)); + expect(synthesized.terraform.required_providers.random.source).toBe( + "hashicorp/random", + ); + }); + + test("deploys with the provider from the OpenTofu registry", async () => { + const output = await driver.deploy([stack]); + expect(output).not.toContain("registry.terraform.io"); + expect(lockFile()).toContain( + 'provider "registry.opentofu.org/hashicorp/random"', + ); + }, 300_000); + + test("reports the version the provider was generated at", async () => { + const { stdout } = await driver.exec("cdktn", [ + "provider", + "list", + "--json", + ]); + const list = JSON.parse(stdout.slice(stdout.indexOf("{"))); + expect(list.local).toEqual([ + expect.objectContaining({ + providerName: "random", + providerVersion: "3.6.0", + }), + ]); + }); + + // Exercises the lock-file comparison: the bump has to be detected as needing + // `init -upgrade`, which plain `init` would reject. + test("upgrades the provider and redeploys at the new version", async () => { + await driver.exec("cdktn", ["provider", "upgrade", "random@=3.6.1"]); + const versions = JSON.parse(driver.readLocalFile(".gen/versions.json")); + expect(providerVersion(versions, "hashicorp/random")).toBe("3.6.1"); + + await driver.deploy([stack]); + expect(lockFile()).toMatch(/version\s+=\s+"3\.6\.1"/); + }, 500_000); +}); diff --git a/test/typescript/provider-add-command/test.ts b/test/typescript/provider-add-command/test.ts index 826d687ff..65e32960c 100644 --- a/test/typescript/provider-add-command/test.ts +++ b/test/typescript/provider-add-command/test.ts @@ -5,6 +5,7 @@ import { onPosix, onWindows, packageJsonWithDependency, + providerVersion, } from "../../test-helper"; describe("provider add command", () => { @@ -83,9 +84,9 @@ describe("provider add command", () => { driver.readLocalFile(".gen/versions.json"), ); - expect( - genVersionsFile["registry.terraform.io/hashicorp/local"], - ).toEqual("2.2.3"); + expect(providerVersion(genVersionsFile, "hashicorp/local")).toEqual( + "2.2.3", + ); }, 240_000, ); @@ -114,9 +115,9 @@ describe("provider add command", () => { driver.readLocalFile(".gen/versions.json"), ); - expect( - genVersionsFile["registry.terraform.io/hashicorp/local"], - ).toEqual("2.2.3"); + expect(providerVersion(genVersionsFile, "hashicorp/local")).toEqual( + "2.2.3", + ); }, 120_000, ); diff --git a/test/typescript/provider-upgrade-command/test.ts b/test/typescript/provider-upgrade-command/test.ts index 4c2fa84da..cf8599c4f 100644 --- a/test/typescript/provider-upgrade-command/test.ts +++ b/test/typescript/provider-upgrade-command/test.ts @@ -5,6 +5,7 @@ import { onPosix, onWindows, packageJsonWithDependency, + providerVersion, } from "../../test-helper"; describe("provider upgrade command", () => { @@ -87,7 +88,7 @@ describe("provider upgrade command", () => { driver.readLocalFile(".gen/versions.json"), ); - expect(genVersionsFile["registry.terraform.io/hashicorp/random"]).toEqual( + expect(providerVersion(genVersionsFile, "hashicorp/random")).toEqual( "3.2.0", ); }); @@ -108,9 +109,9 @@ describe("provider upgrade command", () => { driver.readLocalFile(".gen/versions.json"), ); - expect( - genVersionsFile["registry.terraform.io/hashicorp/random"], - ).toEqual("3.2.0"); + expect(providerVersion(genVersionsFile, "hashicorp/random")).toEqual( + "3.2.0", + ); }, 120_000, ); diff --git a/tools/build-test-matrix.mjs b/tools/build-test-matrix.mjs index 783938150..4a3f73a94 100644 --- a/tools/build-test-matrix.mjs +++ b/tools/build-test-matrix.mjs @@ -42,10 +42,26 @@ const availableVersions = { * @type {Record>} */ const pinnedRuntimes = { + // Declares an OpenTofu-only target, so only the OpenTofu CLI is a meaningful pairing. + "typescript/opentofu-target/test.ts": [ + { product: "opentofu", version: "1.12.6" }, + ], "typescript/provider-features/test.ts": [ { product: "terraform", version: "1.16.1" }, { product: "opentofu", version: "1.12.6" }, ], + // Provider commands driven by the OpenTofu CLI. Their projects declare no targetVersions, so registry selection + // stays Terraform-based; opentofu-target covers that. The Terraform entries restate the `tested` default. + "typescript/provider-add-command/test.ts": [ + { product: "terraform", version: "1.5.7" }, + { product: "terraform", version: "1.16.1" }, + { product: "opentofu", version: "1.12.6" }, + ], + "typescript/provider-upgrade-command/test.ts": [ + { product: "terraform", version: "1.5.7" }, + { product: "terraform", version: "1.16.1" }, + { product: "opentofu", version: "1.12.6" }, + ], }; /** @@ -84,8 +100,12 @@ const absoluteTargets = execFileSync( .map((line) => line.trim()) .filter(Boolean); -const testDirPrefix = `${testDir}/`; +/** Matrix keys are posix-style, so normalise the separators jest emits on Windows. */ +const toPosix = (p) => p.replace(/\\/g, "/"); + +const testDirPrefix = `${toPosix(testDir)}/`; const targets = absoluteTargets + .map(toPosix) .map((p) => (p.startsWith(testDirPrefix) ? p.slice(testDirPrefix.length) : p)) .sort();