Merge pull request juju#641 from alesstimec/jaas-integration-fix

fix: jaas integration test

Author: alesstimec

.github/workflows/test_integration.yml

@@ -103,7 +103,7 @@ jobs:
           TF_ACC: "1"
           TEST_CLOUD: ${{ matrix.action-operator.cloud }}
         run: go test -parallel 1 -timeout 60m -v -cover ./internal/provider/
-        timeout-minutes: 40
+        timeout-minutes: 60
 
   # Run acceptance tests in a matrix with Terraform CLI versions
   add-machine-test:
@@ -189,5 +189,5 @@ jobs:
 
           echo "Running the test"
           cd ./internal/provider/
-          go test ./... -timeout 30m -v -test.run TestAcc_ResourceMachine_AddMachine
-        timeout-minutes: 40
+          go test ./... -timeout 60m -v -test.run TestAcc_ResourceMachine_AddMachine
+        timeout-minutes: 60

.github/workflows/test_integration_jaas.yaml

@@ -41,7 +41,7 @@ jobs:
   test:
     name: Integration-JAAS
     needs: build
-    runs-on: ubuntu-latest
+    runs-on: [self-hosted, jammy, x64]
     strategy:
       fail-fast: false
     timeout-minutes: 60
@@ -55,6 +55,10 @@ jobs:
         with:
           terraform_version: "1.9.*"
           terraform_wrapper: false
+      - name: Install docker compose plugin
+        run: |
+          for pkg in docker.io docker-doc docker-compose docker-compose-v2 podman-docker containerd runc; do sudo apt-get remove -y $pkg; done
+          sudo snap install docker --channel latest/stable
       # Starting JAAS will start the JIMM controller and dependencies and create a Juju controller on LXD and connect it to JIMM.
       - name: Setup JAAS
         uses: canonical/jimm/.github/actions/test-server@v3
@@ -68,11 +72,13 @@ jobs:
           sudo snap install microk8s --channel=1.28-strict/stable
           sudo usermod -a -G snap_microk8s $USER
           sudo chown -R $USER ~/.kube
-          sudo microk8s.enable dns storage
-          sudo microk8s.enable dns local-storage
+          sudo microk8s.enable dns
+          sudo microk8s.enable storage
+          sudo microk8s.enable hostpath-storage
           sudo -g snap_microk8s -E microk8s status --wait-ready --timeout=600
+          sudo microk8s.config view | tee /home/$USER/microk8s-config.yaml
           echo "MICROK8S_CONFIG<<EOF" >> $GITHUB_ENV
-          sudo microk8s.config view >> $GITHUB_ENV
+          echo "$(cat /home/${USER}/microk8s-config.yaml)" >> $GITHUB_ENV
           echo "EOF" >> $GITHUB_ENV
       - name: Create additional networks when testing with LXD
         run: |
@@ -97,5 +103,6 @@ jobs:
       - env:
           TF_ACC: "1"
           TEST_CLOUD: "lxd"
-        run: go test -parallel 10 -timeout 40m -v -cover ./internal/provider/
-        timeout-minutes: 40
+        run: go test -parallel 1 -timeout 60m -v -cover ./internal/provider/
+        timeout-minutes: 60
+

docs/resources/kubernetes_cloud.md

@@ -37,8 +37,8 @@ resource "juju_model" "my-model" {
 ### Optional
 
 - `kubernetes_config` (String, Sensitive) The kubernetes config file path for the cloud. Cloud credentials will be added to the Juju controller for you.
-- `parent_cloud_name` (String) The parent cloud name in case adding k8s cluster from existed cloud. Changing this value will cause the cloud to be destroyed and recreated by terraform.
-- `parent_cloud_region` (String) The parent cloud region name in case adding k8s cluster from existed cloud. Changing this value will cause the cloud to be destroyed and recreated by terraform.
+- `parent_cloud_name` (String) The parent cloud name, for adding a k8s cluster from an existing cloud. Changing this value will cause the cloud to be destroyed and recreated by terraform. *Note* that this value must be set when running against a JAAS controller.
+- `parent_cloud_region` (String) The parent cloud region name, for adding a k8s cluster from an existing cloud. Changing this value will cause the cloud to be destroyed and recreated by terraform. *Note* that this value must be set when running against a JAAS controller.
 
 ### Read-Only
 

internal/provider/resource_kubernetes_cloud.go

@@ -20,6 +20,7 @@ import (
 // Ensure provider defined types fully satisfy framework interfaces.
 var _ resource.Resource = &kubernetesCloudResource{}
 var _ resource.ResourceWithConfigure = &kubernetesCloudResource{}
+var _ resource.ResourceWithConfigValidators = &kubernetesCloudResource{}
 
 func NewKubernetesCloudResource() resource.Resource {
 	return &kubernetesCloudResource{}
@@ -62,6 +63,13 @@ func (r *kubernetesCloudResource) Configure(ctx context.Context, req resource.Co
 	r.subCtx = tflog.NewSubsystem(ctx, LogResourceKubernetesCloud)
 }
 
+// ConfigValidators returns a list of functions which will all be performed during validation.
+func (r *kubernetesCloudResource) ConfigValidators(context.Context) []resource.ConfigValidator {
+	return []resource.ConfigValidator{
+		&kuberenetesCloudJAASValidator{r.client},
+	}
+}
+
 // Metadata returns the metadata for the kubernetes cloud resource.
 func (r *kubernetesCloudResource) Metadata(_ context.Context, req resource.MetadataRequest, resp *resource.MetadataResponse) {
 	resp.TypeName = req.ProviderTypeName + "_kubernetes_cloud"
@@ -92,11 +100,11 @@ func (r *kubernetesCloudResource) Schema(_ context.Context, req resource.SchemaR
 				Sensitive:   true,
 			},
 			"parent_cloud_name": schema.StringAttribute{
-				Description: "The parent cloud name in case adding k8s cluster from existed cloud. Changing this value will cause the cloud to be destroyed and recreated by terraform.",
+				Description: "The parent cloud name, for adding a k8s cluster from an existing cloud. Changing this value will cause the cloud to be destroyed and recreated by terraform. *Note* that this value must be set when running against a JAAS controller.",
 				Optional:    true,
 			},
 			"parent_cloud_region": schema.StringAttribute{
-				Description: "The parent cloud region name in case adding k8s cluster from existed cloud. Changing this value will cause the cloud to be destroyed and recreated by terraform.",
+				Description: "The parent cloud region name, for adding a k8s cluster from an existing cloud. Changing this value will cause the cloud to be destroyed and recreated by terraform. *Note* that this value must be set when running against a JAAS controller.",
 				Optional:    true,
 			},
 			"id": schema.StringAttribute{
@@ -128,8 +136,10 @@ func (r *kubernetesCloudResource) Create(ctx context.Context, req resource.Creat
 	// Create the kubernetes cloud.
 	cloudCredentialName, err := r.client.Clouds.CreateKubernetesCloud(
 		&juju.CreateKubernetesCloudInput{
-			Name:             plan.CloudName.ValueString(),
-			KubernetesConfig: plan.KubernetesConfig.ValueString(),
+			Name:              plan.CloudName.ValueString(),
+			KubernetesConfig:  plan.KubernetesConfig.ValueString(),
+			ParentCloudName:   plan.ParentCloudName.ValueString(),
+			ParentCloudRegion: plan.ParentCloudRegion.ValueString(),
 		},
 	)
 	if err != nil {
@@ -201,8 +211,10 @@ func (r *kubernetesCloudResource) Update(ctx context.Context, req resource.Updat
 	// Update the kubernetes cloud.
 	err := r.client.Clouds.UpdateKubernetesCloud(
 		juju.UpdateKubernetesCloudInput{
-			Name:             plan.CloudName.ValueString(),
-			KubernetesConfig: plan.KubernetesConfig.ValueString(),
+			Name:              plan.CloudName.ValueString(),
+			KubernetesConfig:  plan.KubernetesConfig.ValueString(),
+			ParentCloudName:   plan.ParentCloudName.ValueString(),
+			ParentCloudRegion: plan.ParentCloudRegion.ValueString(),
 		},
 	)
 	if err != nil {
@@ -250,6 +262,41 @@ func (r *kubernetesCloudResource) trace(msg string, additionalFields ...map[stri
 	tflog.SubsystemTrace(r.subCtx, LogResourceKubernetesCloud, msg, additionalFields...)
 }
 
+type kuberenetesCloudJAASValidator struct {
+	client *juju.Client
+}
+
+// Description implements the Description method of the resource.ConfigValidator interface.
+func (v *kuberenetesCloudJAASValidator) Description(ctx context.Context) string {
+	return v.MarkdownDescription(ctx)
+}
+
+// MarkdownDescription implements the MarkdownDescription method of the resource.ConfigValidator interface.
+func (v *kuberenetesCloudJAASValidator) MarkdownDescription(_ context.Context) string {
+	return "Enforces that the parent_cloud_name is specified when applying to a JAAS controller."
+}
+
+// ValidateResource implements the ValidateResource method of the resource.ConfigValidator interface.
+func (v *kuberenetesCloudJAASValidator) ValidateResource(ctx context.Context, req resource.ValidateConfigRequest, resp *resource.ValidateConfigResponse) {
+	if v.client == nil {
+		return
+	}
+
+	if !v.client.IsJAAS() {
+		return
+	}
+
+	var data kubernetesCloudResourceModel
+	resp.Diagnostics.Append(req.Config.Get(ctx, &data)...)
+	if resp.Diagnostics.HasError() {
+		return
+	}
+
+	if data.ParentCloudName.ValueString() == "" {
+		resp.Diagnostics.AddError("Plan Error", "Field `parent_cloud_name` must be specified when applying to a JAAS controller.")
+	}
+}
+
 func newKubernetesCloudID(kubernetesCloudName string, cloudCredentialName string) string {
 	return fmt.Sprintf("%s:%s", kubernetesCloudName, cloudCredentialName)
 }

internal/provider/resource_kubernetes_cloud_test.go

@@ -5,6 +5,7 @@ package provider
 
 import (
 	"os"
+	"regexp"
 	"testing"
 
 	"github.com/hashicorp/terraform-plugin-testing/helper/acctest"
@@ -24,12 +25,17 @@ func TestAcc_ResourceKubernetesCloud(t *testing.T) {
 	cloudName := acctest.RandomWithPrefix("tf-test-k8scloud")
 	cloudConfig := os.Getenv("MICROK8S_CONFIG")
 
+	jaasTest := false
+	if _, ok := os.LookupEnv("IS_JAAS"); ok {
+		jaasTest = true
+	}
+
 	resource.ParallelTest(t, resource.TestCase{
 		PreCheck:                 func() { testAccPreCheck(t) },
 		ProtoV6ProviderFactories: frameworkProviderFactories,
 		Steps: []resource.TestStep{
 			{
-				Config: testAccResourceKubernetesCloudWithoutModel(cloudName, cloudConfig),
+				Config: testAccResourceKubernetesCloudWithoutModel(cloudName, cloudConfig, jaasTest),
 				Check: resource.ComposeTestCheckFunc(
 					resource.TestCheckResourceAttr("juju_kubernetes_cloud."+cloudName, "name", cloudName),
 				),
@@ -38,13 +44,54 @@ func TestAcc_ResourceKubernetesCloud(t *testing.T) {
 	})
 }
 
-func testAccResourceKubernetesCloudWithoutModel(cloudName string, config string) string {
+func TestAcc_ResourceKubernetesCloudWithJAASIncompleteConfig(t *testing.T) {
+	OnlyTestAgainstJAAS(t)
+	// TODO: This test is not adding model as a resource, which is required.
+	// The reason in the race that we (potentially) have in the Juju side.
+	// Once the problem is fixed (https://bugs.launchpad.net/juju/+bug/2084448),
+	// we should add the model as a resource.
+	if testingCloud != LXDCloudTesting {
+		t.Skip(t.Name() + " only runs with LXD")
+	}
+	cloudName := acctest.RandomWithPrefix("tf-test-k8scloud")
+	cloudConfig := os.Getenv("MICROK8S_CONFIG")
+
+	resource.ParallelTest(t, resource.TestCase{
+		PreCheck:                 func() { testAccPreCheck(t) },
+		ProtoV6ProviderFactories: frameworkProviderFactories,
+		Steps: []resource.TestStep{{
+			Config:      testAccResourceKubernetesCloudWithoutParentCloudName(cloudName, cloudConfig),
+			ExpectError: regexp.MustCompile("Field `parent_cloud_name` must be specified when applying to a JAAS.*"),
+		}},
+	})
+}
+
+func testAccResourceKubernetesCloudWithoutModel(cloudName string, config string, jaasTest bool) string {
 	return internaltesting.GetStringFromTemplateWithData(
 		"testAccResourceSecret",
 		`
 resource "juju_kubernetes_cloud" "{{.CloudName}}" {
  name = "{{.CloudName}}"
  kubernetes_config = file("~/microk8s-config.yaml")
+ {{ if .JAASTest }}
+ parent_cloud_name = "lxd"
+ parent_cloud_region = "localhost"
+ {{ end }}
+}
+`, internaltesting.TemplateData{
+			"CloudName": cloudName,
+			"Config":    config,
+			"JAASTest":  jaasTest,
+		})
+}
+
+func testAccResourceKubernetesCloudWithoutParentCloudName(cloudName string, config string) string {
+	return internaltesting.GetStringFromTemplateWithData(
+		"testAccResourceSecret",
+		`
+resource "juju_kubernetes_cloud" "{{.CloudName}}" {
+ name = "{{.CloudName}}"
+ kubernetes_config = "test config"
 }
 `, internaltesting.TemplateData{
 			"CloudName": cloudName,