fix: name space imports should not change project

.github/workflows/pull_request.yaml

@@ -66,10 +66,12 @@ jobs:
       - name: shell check
         shell: /home/runner/.nix-profile/bin/nix develop --ignore-environment --extra-experimental-features nix-command --extra-experimental-features flakes --keep HOME --keep SSH_AUTH_SOCK --keep GITHUB_TOKEN --keep AWS_ROLE --keep AWS_REGION --keep AWS_DEFAULT_REGION --keep AWS_ACCESS_KEY_ID --keep AWS_SECRET_ACCESS_KEY --keep AWS_SESSION_TOKEN --keep UPDATECLI_GPGTOKEN --keep UPDATECLI_GITHUB_TOKEN --keep UPDATECLI_GITHUB_ACTOR --keep GPG_SIGNING_KEY --keep NIX_SSL_CERT_FILE --keep NIX_ENV_LOADED --keep TERM --command bash -e {0}
         run: |
-          while read -r file; do
-            echo "checking $file..."
-            shellcheck -x "$file"
-          done <<<"$(grep -Rl -e '^#!' | grep -v '.terraform'| grep -v '.git')"
+          # while read -r file; do
+          #   echo "checking $file..."
+          #   shellcheck -x "$file"
+          # done <<<"$(grep -Rl -e '^#!' | grep -v '.terraform'| grep -v '.git')"
+          # in the future run this on every script, but first we need to eliminate unused scripts
+          shellcheck -x "./scripts/run_tests.sh"
 
   validate-commit-message:
     runs-on: ubuntu-latest

.github/workflows/release.yaml

@@ -44,8 +44,8 @@ jobs:
           echo '${{ env.GPG_PASSPHRASE }}' | gpg --detach-sig --pinentry-mode loopback --passphrase-fd 0 --output "${SHASUM_FILE}.sig" --sign "${SHASUM_FILE}"
 
           echo "Validating signature..."
-          gpg --verify "${SHASUM_FILE}.sig" "${SHASUM_FILE}"
-          if [ $? -eq 0 ]; then
+
+          if ! gpg --verify "${SHASUM_FILE}.sig" "${SHASUM_FILE}"; then
             echo "Signature is valid..."
           else
             echo "Signature verification failed!"

examples/state_import/main.tf

@@ -47,7 +47,8 @@ locals {
   email                 = (var.email != "" ? var.email : "${local.identifier}@${local.zone}")
   acme_server_url       = "https://acme-v02.api.letsencrypt.org"
   cluster_name          = "tf-all-in-one-config"
-  project_id            = data.rancher2_cluster.downstream_cluster.default_project_id
+  project_mismatch      = var.project_mismatch # if this is true, then the import should fail
+  project_id            = (local.project_mismatch ? rancher2_project.test.id : data.rancher2_cluster.downstream_cluster.default_project_id)
   # tflint-ignore: terraform_unused_declarations
   fail_project_id = (strcontains(local.project_id, ":") != true ? one([local.project_id, "project_id_malformed"]) : false)
 }
@@ -182,18 +183,31 @@ resource "rancher2_namespace" "test" {
   }
 }
 
+resource "rancher2_project" "test" {
+  depends_on = [
+    module.rancher,
+    module.rke2_image,
+    module.downstream_cluster,
+    data.rancher2_cluster.downstream_cluster,
+    rancher2_namespace.test,
+  ]
+  name       = "test"
+  cluster_id = data.rancher2_cluster.downstream_cluster.id
+}
+
 resource "local_file" "import_main" {
   depends_on = [
     module.rancher,
     module.rke2_image,
     module.downstream_cluster,
     data.rancher2_cluster.downstream_cluster,
     rancher2_namespace.test,
+    rancher2_project.test,
   ]
   filename = "${local.tf_data_dir}/tf-rancher-imported/main.tf"
   content = templatefile("${path.module}/modules/import/main.tftpl", {
     cluster_id   = module.downstream_cluster.cluster_id
-    namespace_id = "${local.project_id}.${rancher2_namespace.test.id}"
+    namespace_id = join(".", [local.project_id, rancher2_namespace.test.id])
   })
 }
 
@@ -221,6 +235,7 @@ module "import" {
     zone                = "${local.zone}"
     machine_config_kind = "${module.downstream_cluster.machine_config_kind}"
     machine_config_name = "${module.downstream_cluster.machine_config_name}"
+    project_mismatch    = "${local.project_mismatch}"
   EOT
   skip_destroy = true  // this is for testing purposes, it prevents an issue where the imported resources destroy the API objects and the main resources error out on destroy (not found)
   init         = false // this is for testing purposes, it allow us to use dev overrides in the terraformrc to use the locally built binary rather than the registry provider

examples/state_import/modules/import/variables.tf

@@ -1,24 +1,36 @@
+# tflint-ignore: terraform_unused_declarations
 variable "cluster_name" {
   type = string
 }
+# tflint-ignore: terraform_unused_declarations
 variable "rke2_version" {
   type = string
 }
+# tflint-ignore: terraform_unused_declarations
 variable "node_count" {
   type = number
 }
+# tflint-ignore: terraform_unused_declarations
 variable "rancher_key" {
   type = string
 }
+# tflint-ignore: terraform_unused_declarations
 variable "domain" {
   type = string
 }
+# tflint-ignore: terraform_unused_declarations
 variable "zone" {
   type = string
 }
+# tflint-ignore: terraform_unused_declarations
 variable "machine_config_kind" {
   type = string
 }
+# tflint-ignore: terraform_unused_declarations
 variable "machine_config_name" {
   type = string
 }
+# tflint-ignore: terraform_unused_declarations
+variable "project_mismatch" {
+  type = string
+}

examples/state_import/variables.tf

@@ -100,3 +100,12 @@ variable "data_path" {
   EOT
   default     = null
 }
+variable "project_mismatch" {
+  type        = string
+  description = <<-EOT
+    This is used for testing, it ensures that namespace imports can't alter the namespace.
+    In previous versions it was possible to move the namespace between projects as you import it, this is no longer possible.
+    Setting this to true should cause the import module to fail.
+  EOT
+  default     = false
+}

rancher2/import_rancher2_namespace.go

@@ -1,39 +1,34 @@
 package rancher2
 
 import (
+	"fmt"
 	"log"
 
 	"github.com/hashicorp/terraform-plugin-sdk/helper/schema"
-	clusterClient "github.com/rancher/rancher/pkg/client/generated/cluster/v3"
 )
 
 func resourceRancher2NamespaceImport(d *schema.ResourceData, meta interface{}) ([]*schema.ResourceData, error) {
+	// in this context the clusterID is rancher2_cluster_v2.cluster_v1_id or rancher2_cluster.id
+	// the input is expected to be "project_id.namespace_id"
+	// project_id is a concatenation of "cluster_id:project_id" or data.rancher2_cluster.downstream_cluster.default_project_id
+	log.Printf("[INFO] Splitting given id %s", d.Id())
 	projectID, resourceID := splitID(d.Id())
-
+	log.Printf("[INFO] Splitting project %s", projectID)
 	clusterID, projectID := splitProjectID(projectID)
+	log.Printf("[INFO] Using cluster id %s", clusterID)
 
 	client, err := meta.(*Config).ClusterClient(clusterID)
 	if err != nil {
+		log.Printf("[INFO] Problem getting cluster client for cluster with id \"%s\"", clusterID)
 		return []*schema.ResourceData{}, err
 	}
 
 	ns, err := client.Namespace.ByID(resourceID)
 	if err != nil {
 		return []*schema.ResourceData{}, err
 	}
-
-	d.Set("project_id", clusterID)
-	if projectID != "" {
-		log.Printf("[INFO] Moving Namespace ID %s to project %s", d.Id(), projectID)
-		nsMove := &clusterClient.NamespaceMove{
-			ProjectID: projectID,
-		}
-
-		err = client.Namespace.ActionMove(ns, nsMove)
-		if err != nil {
-			return []*schema.ResourceData{}, err
-		}
-		d.Set("project_id", projectID)
+	if ns.ProjectID != projectID {
+		return []*schema.ResourceData{}, fmt.Errorf("[ERROR] Project ID \"%s\" in import object doesn't match resource to import (\"%s\").", projectID, ns.ProjectID)
 	}
 
 	err = flattenNamespace(d, ns)

rancher2/util.go

@@ -395,6 +395,8 @@ func splitBySep(data, sep string) []string {
 	return strings.Split(data, sep)
 }
 
+// eg. "abc123.def456"(id) returns "abc123"(clusterID), "def456"(resourceID)
+// eg. "abc123"(id) returns ""(clusterID), "abc123"(resourceID)
 func splitID(id string) (clusterID, resourceID string) {
 	separator := "."
 
@@ -437,6 +439,8 @@ func splitProjectIDPart(id string) (projectID string) {
 	return ""
 }
 
+// eg. "abc123:def456"(id) would return "abc123"(clusterID),"abc123:def456"(projectID)
+// eg. "abc123"(id) would return "abc123"(clusterID),""(projectID)
 func splitProjectID(id string) (clusterID, projectID string) {
 	id = strings.TrimSuffix(id, clusterProjectIDSeparator)
 

scripts/run_tests.sh

@@ -4,7 +4,7 @@ get_git_root() {
   git rev-parse --show-toplevel
 }
 
-if get_git_root; then cd "$(get_git_root)"; else echo "expects to be run from within the terraform provider git repo"; exit 1; fi
+if get_git_root; then cd "$(get_git_root)" || exit; else echo "expects to be run from within the terraform provider git repo"; exit 1; fi
 export REPO_ROOT=$PWD
 
 IDENTIFIER=""
@@ -135,23 +135,25 @@ echo 'if tmp directory is missing, try restarting dev environment'
 if [ -z "$cleanup_id" ]; then
   if [ "false" = "$skip_build" ]; then
     echo 'building...'
-    $REPO_ROOT/scripts/gobuild.sh
+    if ! "$REPO_ROOT/scripts/gobuild.sh"; then C=$?; echo "failed to compile provider, exit code $C"; exit $C; fi
     export SKIP_BUILD="false"
+    echo "provider successfully compiles..."
   else
     echo "skipping build..."
     export SKIP_BUILD="true"
   fi
 
   # Test if tests can compile
   echo "checking tests for compile errors..."
-  cd "$REPO_ROOT/test"
+  cd "$REPO_ROOT/test" || exit
   if ! go mod tidy; then C=$?; echo "failed to tidy, exit code $C"; exit $C; fi
 
-  for file in $(find $REPO_ROOT/test -name '*.go'); do
+  while read -r file; do
     echo "found $file";
     if ! go test -c "$file" -o "$file.test"; then C=$?; echo "failed to compile $file, exit code $C"; exit $C; fi
     rm -rf "$file.test"
-  done
+  done <<<"$(find "$REPO_ROOT/test" -name '*.go')"
+
   echo "compile checks passed..."
 
   # Run tests initially

test/downstream/downstream_test.go

@@ -351,7 +351,19 @@ func TestDownstreamImport(t *testing.T) {
 		Upgrade:                  true,
 	})
 
+	terraformOptions.Vars["project_mismatch"] = "true"
 	_, err = terraform.InitAndApplyE(t, terraformOptions)
+	if err == nil {
+		t.Log("Test failed, tearing down...")
+		util.GetErrorLogs(t, testDir+"/kubeconfig")
+		util.Teardown(t, testDir, terraformOptions, keyPair)
+		os.Remove(exampleDir + ".terraform.lock.hcl")
+		sshAgent.Stop()
+		t.Fatalf("Error creating cluster: %s", err)
+	}
+
+	terraformOptions.Vars["project_mismatch"] = "false"
+	_, err = terraform.ApplyE(t, terraformOptions)
 	if err != nil {
 		t.Log("Test failed, tearing down...")
 		util.GetErrorLogs(t, testDir+"/kubeconfig")
@@ -362,11 +374,13 @@ func TestDownstreamImport(t *testing.T) {
 	}
 	util.CheckReady(t, testDir+"/kubeconfig")
 	util.CheckRunning(t, testDir+"/kubeconfig")
+
 	if t.Failed() {
 		t.Log("Test failed...")
 	} else {
 		t.Log("Test passed...")
 	}
+
 	util.Teardown(t, testDir, terraformOptions, keyPair)
 	os.Remove(exampleDir + "/.terraform.lock.hcl")
 	os.Remove(exampleDir + "/TF_DATA_DIR.env")