Skip to content

Commit

Permalink
fix: kubernetes cloud resource
Browse files Browse the repository at this point in the history
  • Loading branch information
alesstimec committed Dec 17, 2024
1 parent 54f8588 commit ee0178a
Show file tree
Hide file tree
Showing 5 changed files with 117 additions and 22 deletions.
6 changes: 3 additions & 3 deletions .github/workflows/test_integration.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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
7 changes: 4 additions & 3 deletions .github/workflows/test_integration_jaas.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ jobs:
# Ensure project builds before running test
build:
name: Build-JAAS
runs-on: [self-hosted, jammy, x64]
runs-on: ubuntu-latest
timeout-minutes: 5
steps:
- uses: actions/checkout@v4
Expand Down Expand Up @@ -103,5 +103,6 @@ jobs:
- env:
TF_ACC: "1"
TEST_CLOUD: "lxd"
run: go test -parallel 1 -timeout 40m -v -cover ./internal/provider/
timeout-minutes: 40
run: go test -parallel 1 -timeout 60m -v -cover ./internal/provider/
timeout-minutes: 60

4 changes: 2 additions & 2 deletions docs/resources/kubernetes_cloud.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 in case adding k8s cluster from existed 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 in case adding k8s cluster from existed 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

Expand Down
63 changes: 57 additions & 6 deletions internal/provider/resource_kubernetes_cloud.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{}
Expand Down Expand Up @@ -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"
Expand Down Expand Up @@ -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 in case adding k8s cluster from existed 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 in case adding k8s cluster from existed 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{
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -250,6 +262,45 @@ 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 this resource can only be used with JAAS"
}

// 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", "parent_cloud_name must be specified when applying to a JAAS controller")
}

if data.ParentCloudRegion.ValueString() == "" {
resp.Diagnostics.AddError("Plan Error", "parent_cloud_region must be specified when applying to a JAAS controller")
}
}

func newKubernetesCloudID(kubernetesCloudName string, cloudCredentialName string) string {
return fmt.Sprintf("%s:%s", kubernetesCloudName, cloudCredentialName)
}
59 changes: 51 additions & 8 deletions internal/provider/resource_kubernetes_cloud_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ package provider

import (
"os"
"regexp"
"testing"

"github.com/hashicorp/terraform-plugin-testing/helper/acctest"
Expand All @@ -14,12 +15,6 @@ import (
)

func TestAcc_ResourceKubernetesCloud(t *testing.T) {
// Note (alesstimec): Skipping this test, because the default
// hosted cloud tf provider adds is "other", which cannot
// be parsed by JIMM - it needs a valid cloud/region to determine
// which controller to add the cloud to.
SkipJAAS(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),
Expand All @@ -30,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),
),
Expand All @@ -44,13 +44,56 @@ 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: testAccResourceKubernetesCloudWithoutParentCloud(cloudName, cloudConfig),
ExpectError: regexp.MustCompile("parent_cloud_region must be specified when applying to a JAAS controller"),
},
},
})
}

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 testAccResourceKubernetesCloudWithoutParentCloud(cloudName string, config string) string {
return internaltesting.GetStringFromTemplateWithData(
"testAccResourceSecret",
`
resource "juju_kubernetes_cloud" "{{.CloudName}}" {
name = "{{.CloudName}}"
kubernetes_config = "test config"
}
`, internaltesting.TemplateData{
"CloudName": cloudName,
Expand Down

0 comments on commit ee0178a

Please sign in to comment.