fix: Ensure Version field is handled correctly when bootstrapping TF module Promise

.github/workflows/tests.yaml

@@ -21,6 +21,14 @@ jobs:
           check-latest: true
       - name: Install Terraform CLI
         uses: hashicorp/setup-terraform@v3
+      - name: Setup SSH Agent
+        uses: webfactory/[email protected]
+        with:
+          ssh-private-key: ${{ secrets.SYNTASSO_KRATIX_CLI_PRIVATE_TF_MODULE_TEST_FIXTURE_REPO_DEPLOY_KEY }}
+      - name: Setup SSH
+        run: |
+          mkdir -p ~/.ssh
+          ssh-keyscan github.com >> ~/.ssh/known_hosts
       - name: Run make test
         run: make test
       - name: Run govulncheck
@@ -55,4 +63,4 @@ jobs:
           release-please \
             --token=$TOKEN \
             --repo-url=syntasso/kratix-cli \
-            release-pr
+            release-pr

cmd/init_tf_module_promise.go

@@ -39,15 +39,15 @@ To pull modules from private registries, ensure your system is logged in to the
   # Initialize a Promise from a Terraform Module in Terraform registry
   kratix init tf-module-promise iam \
   	--module-source terraform-aws-modules/iam/aws \
-  	--module-version 6.2.3 \
+  	--module-registry-version 6.2.3 \
   	--group syntasso.io \
 	--kind IAM \
 	--version v1alpha1`,
 		RunE: InitFromTerraformModule,
 		Args: cobra.ExactArgs(1),
 	}
 
-	moduleSource, moduleVersion string
+	moduleSource, moduleRegistryVersion string
 )
 
 func init() {
@@ -56,15 +56,21 @@ func init() {
 		"This can be a Git URL, Terraform registry path, or a local directory path. \n"+
 		"It follows the same format as the `source` argument in the Terraform module block.",
 	)
-	terraformModuleCmd.Flags().StringVarP(&moduleVersion, "module-version", "m", "", "(Optional) version of the terraform module; "+
+	terraformModuleCmd.Flags().StringVarP(&moduleRegistryVersion, "module-registry-version", "r", "", "(Optional) version of the Terraform module from a registry; "+
 		"only use when pulling modules from Terraform registry",
 	)
 	terraformModuleCmd.MarkFlagRequired("module-source")
 }
 
 func InitFromTerraformModule(cmd *cobra.Command, args []string) error {
 	fmt.Println("Fetching terraform module variables, this might take up to a minute...")
-	variables, err := internal.GetVariablesFromModule(moduleSource, moduleVersion)
+
+	if moduleRegistryVersion != "" && !internal.IsTerraformRegistrySource(moduleSource) {
+		fmt.Println("Error: --module-registry-version is only valid for Terraform registry sources like 'namespace/name/provider'. For git URLs (e.g., 'git::https://github.com/org/repo.git?ref=v1.0.0') or local paths, embed the ref directly in --module-source instead.")
+		return fmt.Errorf("invalid use of --module-registry-version with non-registry source")
+	}
+
+	variables, err := internal.GetVariablesFromModule(moduleSource, moduleRegistryVersion)
 	if err != nil {
 		fmt.Printf("Error: failed to download and convert terraform module to CRD: %s\n", err)
 		return nil
@@ -82,16 +88,16 @@ func InitFromTerraformModule(cmd *cobra.Command, args []string) error {
 		return nil
 	}
 
-	resourceConfigure, err := generateTerraformModuleResourceConfigurePipeline()
+	resourceConfigure, err := generateTerraformModuleResourceConfigurePipeline(moduleRegistryVersion)
 	if err != nil {
 		fmt.Printf("Error: failed to generate promise pipelines: %s\n", err)
 		return nil
 	}
 
 	promiseName := args[0]
 	extraFlags := fmt.Sprintf("--module-source %s", moduleSource)
-	if moduleVersion != "" {
-		extraFlags = fmt.Sprintf("%s --module-version %s", extraFlags, moduleVersion)
+	if moduleRegistryVersion != "" {
+		extraFlags = fmt.Sprintf("%s --module-registry-version %s", extraFlags, moduleRegistryVersion)
 	}
 	templateValues, err := generateTemplateValues(promiseName, "tf-module-promise", extraFlags, resourceConfigure, string(crdSchema))
 	if err != nil {
@@ -123,18 +129,18 @@ func InitFromTerraformModule(cmd *cobra.Command, args []string) error {
 	return nil
 }
 
-func generateTerraformModuleResourceConfigurePipeline() (string, error) {
+func generateTerraformModuleResourceConfigurePipeline(moduleRegistryVersion string) (string, error) {
 	envs := []corev1.EnvVar{
 		{
 			Name:  "MODULE_SOURCE",
 			Value: moduleSource,
 		},
 	}
 
-	if moduleVersion != "" {
+	if moduleRegistryVersion != "" {
 		envs = append(envs, corev1.EnvVar{
-			Name:  "MODULE_VERSION",
-			Value: moduleVersion,
+			Name:  "MODULE_REGISTRY_VERSION",
+			Value: moduleRegistryVersion,
 		})
 	}
 
@@ -150,7 +156,7 @@ func generateTerraformModuleResourceConfigurePipeline() (string, error) {
 					"containers": []any{
 						v1alpha1.Container{
 							Name:  "terraform-generate",
-							Image: "ghcr.io/syntasso/kratix-cli/terraform-generate:v0.2.0",
+							Image: "ghcr.io/syntasso/kratix-cli/terraform-generate:v0.4.0",
 							Env:   envs,
 						},
 					},

cmd/kratix/main.go

@@ -21,7 +21,7 @@ import (
 )
 
 // needs to be updated before cutting a new release to desired version and should match the next version in .release-please-manifest.json
-var version = "0.11.0"
+var version = "0.12.0"
 
 func main() {
 	cmd.Execute(version)

internal/terraform_module.go

@@ -28,14 +28,14 @@ var (
 	terraformInit func(dir string) error                    = runTerraformInit
 )
 
-func GetVariablesFromModule(moduleSource, moduleVersion string) ([]TerraformVariable, error) {
+func GetVariablesFromModule(moduleSource, moduleRegistryVersion string) ([]TerraformVariable, error) {
 	tempDir, err := mkdirTemp("", "terraform-module")
 	if err != nil {
 		return nil, fmt.Errorf("failed to create temp directory: %w", err)
 	}
 	defer os.RemoveAll(tempDir)
 
-	if err := writeTerraformModuleConfig(tempDir, moduleSource, moduleVersion); err != nil {
+	if err := writeTerraformModuleConfig(tempDir, moduleSource, moduleRegistryVersion); err != nil {
 		return nil, err
 	}
 
@@ -57,10 +57,10 @@ func GetVariablesFromModule(moduleSource, moduleVersion string) ([]TerraformVari
 	return variables, nil
 }
 
-func writeTerraformModuleConfig(workDir, moduleSource, moduleVersion string) error {
+func writeTerraformModuleConfig(workDir, moduleSource, moduleRegistryVersion string) error {
 	config := fmt.Sprintf("module \"%s\" {\n  source = \"%s\"\n", kratixModuleName, moduleSource)
-	if moduleVersion != "" {
-		config += fmt.Sprintf("  version = \"%s\"\n", moduleVersion)
+	if moduleRegistryVersion != "" && IsTerraformRegistrySource(moduleSource) {
+		config += fmt.Sprintf("  version = \"%s\"\n", moduleRegistryVersion)
 	}
 	config += "}\n"
 	if err := os.WriteFile(filepath.Join(workDir, "main.tf"), []byte(config), 0o644); err != nil {
@@ -109,6 +109,21 @@ func resolveModuleDir(workDir string) (string, error) {
 	return "", fmt.Errorf("module %s not found in terraform module manifest", kratixModuleName)
 }
 
+func IsTerraformRegistrySource(moduleSource string) bool {
+	// Local filepaths
+	if strings.HasPrefix(moduleSource, "./") || strings.HasPrefix(moduleSource, "../") || strings.HasPrefix(moduleSource, "/") {
+		return false
+	}
+
+	// URLs and other schemes
+	if strings.Contains(moduleSource, "://") || strings.Contains(moduleSource, "::") {
+		return false
+	}
+
+	// Otherwise assume it's a registry source if it has at least two slashes
+	return strings.Count(moduleSource, "/") >= 2
+}
+
 func extractVariablesFromVarsFile(filePath string) ([]TerraformVariable, error) {
 	fileContent, err := readFileContent(filePath)
 	if err != nil {

internal/terraform_module_test.go

@@ -132,8 +132,7 @@ variable "list_object_var" {
 				mainContent, err := os.ReadFile(filepath.Join(tempDir, "main.tf"))
 				Expect(err).NotTo(HaveOccurred())
 				expectContent := `module "kratix_target" {
-  source = "git::mock-source.git//subdir"
-  version = "v1.0.0"
+  source = "git::mock-source.git//subdir?ref=v1.0.0"
 }
 `
 				Expect(string(mainContent)).To(Equal(expectContent))
@@ -161,7 +160,7 @@ variable "bool_var" {
 `), 0o644)
 			})
 
-			variables, err := internal.GetVariablesFromModule("git::mock-source.git", "v1.0.0")
+			variables, err := internal.GetVariablesFromModule("git::mock-source.git//subdir?ref=v1.0.0", "")
 			Expect(err).ToNot(HaveOccurred())
 			Expect(variables).To(HaveLen(4))
 
@@ -183,6 +182,37 @@ variable "bool_var" {
 		})
 	})
 
+	Context("when a registry module version is provided separately", func() {
+		BeforeEach(func() {
+			internal.SetTerraformInitFunc(func(dir string) error {
+				mainContent, err := os.ReadFile(filepath.Join(tempDir, "main.tf"))
+				Expect(err).NotTo(HaveOccurred())
+				expectContent := `module "kratix_target" {
+  source = "terraform-aws-modules/iam/aws"
+  version = "6.2.3"
+}
+`
+				Expect(string(mainContent)).To(Equal(expectContent))
+
+				variablesPath := filepath.Join(tempDir, ".terraform", "modules", "kratix_target", "variables.tf")
+				expectManifest(filepath.Join(tempDir, ".terraform", "modules", "modules.json"), ".terraform/modules/kratix_target")
+				Expect(os.MkdirAll(filepath.Dir(variablesPath), 0o755)).To(Succeed())
+				return os.WriteFile(variablesPath, []byte(`
+variable "example_var" {
+  type        = string
+}
+`), 0o644)
+			})
+		})
+
+		It("adds the version to the terraform config", func() {
+			variables, err := internal.GetVariablesFromModule("terraform-aws-modules/iam/aws", "6.2.3")
+			Expect(err).ToNot(HaveOccurred())
+			Expect(variables).To(HaveLen(1))
+			Expect(variables[0].Name).To(Equal("example_var"))
+		})
+	})
+
 	Context("when terraform init fails", func() {
 		It("errors", func() {
 			internal.SetTerraformInitFunc(func(dir string) error {
@@ -210,6 +240,20 @@ variable "bool_var" {
 	})
 })
 
+var _ = Describe("IsTerraformRegistrySource", func() {
+	DescribeTable("registry source detection",
+		func(source string, expected bool) {
+			Expect(internal.IsTerraformRegistrySource(source)).To(Equal(expected))
+		},
+		Entry("registry path", "namespace/name/provider", true),
+		Entry("nested registry path", "foo/bar/baz//bob/banana", true),
+		Entry("git URL", "git::https://github.com/org/repo.git?ref=v1.0.0", false),
+		Entry("local path", "./modules/vpc", false),
+		Entry("absolute path", "/tmp/module", false),
+		Entry("module with scheme", "https://example.com/archive.tgz", false),
+	)
+})
+
 func expectManifest(manifestPath, moduleDir string) {
 	manifest := fmt.Sprintf(`{"Modules":[{"Key":"module.%s","Dir":"%s"}]}`, "kratix_target", moduleDir)
 	Expect(os.MkdirAll(filepath.Dir(manifestPath), 0o755)).To(Succeed())

stages/terraform-module-promise/Dockerfile

@@ -4,8 +4,9 @@ ARG TARGETOS
 WORKDIR /workspace
 COPY go.mod go.mod
 COPY go.sum go.sum
-COPY stages/terraform-module-promise/main.go main.go
 RUN go mod download
+COPY stages/terraform-module-promise/main.go main.go
+COPY internal internal
 RUN CGO_ENABLED=0 GOOS=$TARGETOS GOARCH=$TARGETARCH GO111MODULE=on go build -a -o from-api-to-terraform-module main.go
 
 FROM --platform=${TARGETARCH:-$BUILDPLATFORM} gcr.io/distroless/cc:nonroot

stages/terraform-module-promise/Makefile

@@ -35,4 +35,4 @@ build-and-push: # Build container image and push it to the container registry
 		${BASE_PATH}
 
 build-and-load: build # Build container image and load it into kind
-	kind load docker-image ${IMG_TAG}:${VERSION} --name platform
+	kind load docker-image ${IMG_TAG}:${VERSION} --name platform

stages/terraform-module-promise/main.go

@@ -8,15 +8,19 @@ import (
 	"path/filepath"
 	"strings"
 
+	"github.com/syntasso/kratix-cli/internal"
 	"gopkg.in/yaml.v3"
 )
 
 func main() {
 	yamlFile := GetEnv("KRATIX_INPUT_FILE", "/kratix/input/object.yaml")
 	outputDir := GetEnv("KRATIX_OUTPUT_DIR", "/kratix/output")
 	moduleSource := MustHaveEnv("MODULE_SOURCE")
-	moduleVersion := MustHaveEnv("MODULE_VERSION")
-	modulePath := os.Getenv("MODULE_PATH") // optional
+	moduleRegistryVersion := os.Getenv("MODULE_REGISTRY_VERSION")
+
+	if moduleRegistryVersion != "" && !internal.IsTerraformRegistrySource(moduleSource) {
+		log.Fatalf("MODULE_REGISTRY_VERSION is only valid for Terraform registry sources (e.g., \"namespace/name/provider\"). For git or local sources, embed the ref directly in MODULE_SOURCE (e.g., \"git::https://github.com/org/repo.git?ref=v1.2.3\"). Provided module_source=%q", moduleSource)
+	}
 
 	yamlContent, err := os.ReadFile(yamlFile)
 	if err != nil {
@@ -44,19 +48,18 @@ func main() {
 
 	uniqueFileName := strings.ToLower(fmt.Sprintf("%s_%s_%s", kind, namespace, name))
 
-	source := fmt.Sprintf("%s//%s?ref=%s", moduleSource, modulePath, moduleVersion)
-	if modulePath == "" {
-		source = fmt.Sprintf("%s?ref=%s", moduleSource, moduleVersion)
-	}
-
 	module := map[string]map[string]map[string]any{
 		"module": {
 			uniqueFileName: {
-				"source": source,
+				"source": moduleSource,
 			},
 		},
 	}
 
+	if moduleRegistryVersion != "" {
+		module["module"][uniqueFileName]["version"] = moduleRegistryVersion
+	}
+
 	// Handle spec if it exists
 	if spec, ok := data["spec"].(map[string]any); ok {
 		for key, value := range spec {

stages/terraform-module-promise/test/stage_test.go

@@ -50,6 +50,38 @@ var expectedOutputNoSpec = `{
   }
 }`
 
+var expectedRegistryOutput = `{
+  "module": {
+    "testobject_non-default_test-object": {
+      "source": "terraform-aws-modules/iam/aws",
+      "version": "6.2.3",
+      "strArr": [
+        {
+          "field": "value"
+        }
+      ],
+      "intArr": [
+        1
+      ],
+      "listBool": [
+        true
+      ],
+      "field": "value",
+      "mapWithinMap": {
+        "entryMap": {
+          "entry": "value",
+          "entry2": 2,
+          "entry3": false
+        },
+        "entry": "value",
+        "entry2": 2,
+        "entry3": false
+      },
+      "number": 7
+    }
+  }
+}`
+
 func runWithEnv(envVars map[string]string) *gexec.Session {
 	cmd := exec.Command(binaryPath)
 	for key, value := range envVars {
@@ -74,8 +106,7 @@ var _ = Describe("From TF module to Promise Stage", func() {
 		envVars = map[string]string{
 			"KRATIX_INPUT_FILE": "assets/test-object.yaml",
 			"KRATIX_OUTPUT_DIR": tmpDir,
-			"MODULE_SOURCE":     "git::example.com",
-			"MODULE_VERSION":    "1.0.0",
+			"MODULE_SOURCE":     "git::example.com?ref=1.0.0",
 		}
 
 	})
@@ -104,4 +135,24 @@ var _ = Describe("From TF module to Promise Stage", func() {
 		Expect(err).NotTo(HaveOccurred())
 		Expect(string(output)).To(MatchJSON(expectedOutputNoSpec))
 	})
+
+	It("adds a registry version when provided separately", func() {
+		envVars["MODULE_SOURCE"] = "terraform-aws-modules/iam/aws"
+		envVars["MODULE_REGISTRY_VERSION"] = "6.2.3"
+		session := runWithEnv(envVars)
+		Eventually(session).Should(gexec.Exit())
+		Expect(session.Buffer()).To(gbytes.Say("Terraform JSON configuration written to %s/testobject_non-default_test-object.tf.json", tmpDir))
+		Expect(session).To(gexec.Exit(0))
+		output, err := os.ReadFile(filepath.Join(tmpDir, "testobject_non-default_test-object.tf.json"))
+		Expect(err).NotTo(HaveOccurred())
+		Expect(string(output)).To(MatchJSON(expectedRegistryOutput))
+	})
+
+	It("errors when registry version is used with a non-registry source", func() {
+		envVars["MODULE_REGISTRY_VERSION"] = "9.9.9"
+		session := runWithEnv(envVars)
+		Eventually(session).Should(gexec.Exit())
+		Expect(session.ExitCode()).NotTo(Equal(0))
+		Expect(session.Err).To(gbytes.Say("MODULE_REGISTRY_VERSION is only valid for Terraform registry sources"))
+	})
 })