From fb6b204505a78f2864f67271e0e60be7e4518405 Mon Sep 17 00:00:00 2001 From: Alastair Flynn Date: Wed, 20 Mar 2024 09:48:44 -0400 Subject: [PATCH 1/5] Quote yq directives --- Makefile | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/Makefile b/Makefile index 7187fa9a..0b0cd02f 100644 --- a/Makefile +++ b/Makefile @@ -54,10 +54,10 @@ endif JUJU=juju CONTROLLER=$(shell ${JUJU} whoami | yq .Controller) -CONTROLLER_ADDRESSES="$(shell ${JUJU} show-controller | yq .${CONTROLLER}.details.\"api-endpoints\" | tr -d "[]' "|tr -d '"'|tr -d '\n')" -USERNAME="$(shell cat ~/.local/share/juju/accounts.yaml | yq .controllers.${CONTROLLER}.user|tr -d '"')" -PASSWORD="$(shell cat ~/.local/share/juju/accounts.yaml | yq .controllers.${CONTROLLER}.password|tr -d '"')" -CA_CERT="$(shell ${JUJU} show-controller $(echo ${CONTROLLER}|tr -d '"')| yq .${CONTROLLER}.details.\"ca-cert\"|tr -d '"'|sed 's/\\n/\n/g')" +CONTROLLER_ADDRESSES="$(shell ${JUJU} show-controller | yq '.${CONTROLLER}.details."api-endpoints"' | tr -d "[]' "|tr -d '"'|tr -d '\n')" +USERNAME="$(shell cat ~/.local/share/juju/accounts.yaml | yq '.controllers.${CONTROLLER}.user'|tr -d '"')" +PASSWORD="$(shell cat ~/.local/share/juju/accounts.yaml | yq '.controllers.${CONTROLLER}.password'|tr -d '"')" +CA_CERT="$(shell ${JUJU} show-controller $(echo ${CONTROLLER}|tr -d '"')| yq '.${CONTROLLER}.details."ca-cert"'|tr -d '"'|sed 's/\\n/\n/g')" .PHONY: juju-unit-test juju-unit-test: From ce4fa07147402bc50e5fafeddeb285759e786138 Mon Sep 17 00:00:00 2001 From: Alastair Flynn Date: Thu, 11 Apr 2024 12:23:46 +0100 Subject: [PATCH 2/5] Add validator --- docs/index.md | 4 +-- internal/provider/resource_application.go | 4 +++ internal/provider/validator_base.go | 1 + internal/provider/validator_channel.go | 40 +++++++++++++++++++++++ 4 files changed, 47 insertions(+), 2 deletions(-) create mode 100644 internal/provider/validator_channel.go diff --git a/docs/index.md b/docs/index.md index 55b3e260..55cda702 100644 --- a/docs/index.md +++ b/docs/index.md @@ -90,7 +90,7 @@ Terraform 0.13 and later: terraform { required_providers { juju = { - version = "~> 0.10.0" + version = "~> 0.11.0" source = "juju/juju" } } @@ -149,7 +149,7 @@ resource "juju_integration" "wp_to_percona" { Terraform 0.12 and earlier: ```terraform provider "juju" { - version = "~> 0.10.0" + version = "~> 0.11.0" controller_addresses = "10.225.205.241:17070,10.225.205.242:17070" diff --git a/internal/provider/resource_application.go b/internal/provider/resource_application.go index 8c566415..56fa96c7 100644 --- a/internal/provider/resource_application.go +++ b/internal/provider/resource_application.go @@ -26,6 +26,7 @@ import ( "github.com/hashicorp/terraform-plugin-framework/types" "github.com/hashicorp/terraform-plugin-log/tflog" "github.com/juju/errors" + "github.com/juju/juju/core/constraints" "github.com/juju/terraform-provider-juju/internal/juju" @@ -238,6 +239,9 @@ func (r *applicationResource) Schema(_ context.Context, _ resource.SchemaRequest PlanModifiers: []planmodifier.String{ stringplanmodifier.UseStateForUnknown(), }, + Validators: []validator.String{ + stringIsBaseValidator{}, + }, }, "revision": schema.Int64Attribute{ Description: "The revision of the charm to deploy. During the update phase, the charm revision should be update before config update, to avoid issues with config parameters parsing.", diff --git a/internal/provider/validator_base.go b/internal/provider/validator_base.go index c5942eb5..d0b263a5 100644 --- a/internal/provider/validator_base.go +++ b/internal/provider/validator_base.go @@ -7,6 +7,7 @@ import ( "context" "github.com/hashicorp/terraform-plugin-framework/schema/validator" + "github.com/juju/juju/core/base" ) diff --git a/internal/provider/validator_channel.go b/internal/provider/validator_channel.go new file mode 100644 index 00000000..53eb063e --- /dev/null +++ b/internal/provider/validator_channel.go @@ -0,0 +1,40 @@ +// Copyright 2024 Canonical Ltd. +// Licensed under the AGPLv3, see LICENCE file for details. + +package provider + +import ( + "context" + + "github.com/hashicorp/terraform-plugin-framework/schema/validator" + "github.com/juju/charm/v11" +) + +type stringIsChannelValidator struct{} + +// Description returns a plain text description of the validator's behavior, suitable for a practitioner to understand its impact. +func (v stringIsChannelValidator) Description(context.Context) string { + return "string must conform to track/risk or track/risk/branch e.g. latest/stable" +} + +// MarkdownDescription returns a markdown formatted description of the validator's behavior, suitable for a practitioner to understand its impact. +func (v stringIsChannelValidator) MarkdownDescription(context.Context) string { + return "string must conform to track/risk or track/risk/branch e.g. latest/stable" +} + +// Validate runs the main validation logic of the validator, reading configuration data out of `req` and updating `resp` with diagnostics. +func (v stringIsChannelValidator) ValidateString(_ context.Context, req validator.StringRequest, resp *validator.StringResponse) { + // If the value is unknown or null, there is nothing to validate. + if req.ConfigValue.IsUnknown() || req.ConfigValue.IsNull() { + return + } + + if channel, err := charm.ParseChannel(req.ConfigValue.ValueString()); err != nil || channel.Track == "" || channel.Risk == "" { + resp.Diagnostics.AddAttributeError( + req.Path, + "Invalid Channel", + "String must conform to track/risk or track/risk/branch, e.g. latest/stable", + ) + return + } +} From 6f0d19bdfe1b0a3861dae741eb4c0387e507e964 Mon Sep 17 00:00:00 2001 From: Alastair Flynn Date: Fri, 12 Apr 2024 13:51:49 +0100 Subject: [PATCH 3/5] Run make docs, update version in examples and update env command --- Makefile | 4 +--- examples/provider/provider.tf | 2 +- examples/provider/provider_0.12.tf | 2 +- 3 files changed, 3 insertions(+), 5 deletions(-) diff --git a/Makefile b/Makefile index 0b0cd02f..6c745207 100644 --- a/Makefile +++ b/Makefile @@ -67,9 +67,7 @@ juju-unit-test: .PHONY: envtestlxd envtestlxd: ## envtestlxd: Under development - Include env var and run unit tests against lxd - JUJU_CONTROLLER_ADDRESSES=${CONTROLLER_ADDRESSES} \ - JUJU_USERNAME=${USERNAME} JUJU_PASSWORD=${PASSWORD} \ - JUJU_CA_CERT=${CA_CERT} TF_ACC=1 TEST_CLOUD=lxd go test ./... -v $(TESTARGS) -timeout 120m + JUJU_CONTROLLER_ADDRESSES=${CONTROLLER_ADDRESSES} JUJU_USERNAME=${USERNAME} JUJU_PASSWORD=${PASSWORD} JUJU_CA_CERT=${CA_CERT} TF_ACC=1 TEST_CLOUD=lxd go test ./... -v $(TESTARGS) -timeout 120m .PHONY: testlxd testlxd: diff --git a/examples/provider/provider.tf b/examples/provider/provider.tf index 87cdbd9f..d652d98d 100644 --- a/examples/provider/provider.tf +++ b/examples/provider/provider.tf @@ -1,7 +1,7 @@ terraform { required_providers { juju = { - version = "~> 0.10.0" + version = "~> 0.11.0" source = "juju/juju" } } diff --git a/examples/provider/provider_0.12.tf b/examples/provider/provider_0.12.tf index 0c3f68f3..beccf681 100644 --- a/examples/provider/provider_0.12.tf +++ b/examples/provider/provider_0.12.tf @@ -1,5 +1,5 @@ provider "juju" { - version = "~> 0.10.0" + version = "~> 0.11.0" controller_addresses = "10.225.205.241:17070,10.225.205.242:17070" From 9c475955dc2e11d55ab60d8de407d2a2fb74bed4 Mon Sep 17 00:00:00 2001 From: Alastair Flynn Date: Fri, 12 Apr 2024 13:55:39 +0100 Subject: [PATCH 4/5] Fix errors --- internal/provider/resource_application.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/provider/resource_application.go b/internal/provider/resource_application.go index 56fa96c7..c4151d6c 100644 --- a/internal/provider/resource_application.go +++ b/internal/provider/resource_application.go @@ -240,7 +240,7 @@ func (r *applicationResource) Schema(_ context.Context, _ resource.SchemaRequest stringplanmodifier.UseStateForUnknown(), }, Validators: []validator.String{ - stringIsBaseValidator{}, + stringIsChannelValidator{}, }, }, "revision": schema.Int64Attribute{ From c216fece98d7831da58ed3f1d62294e4b993196d Mon Sep 17 00:00:00 2001 From: Alastair Flynn Date: Thu, 18 Apr 2024 10:45:18 +0100 Subject: [PATCH 5/5] Add unit tests --- Makefile | 4 +- go.mod | 1 + go.sum | 2 + internal/provider/resource_application.go | 2 +- internal/provider/validator_channel.go | 17 +++-- internal/provider/validator_channel_test.go | 73 +++++++++++++++++++++ 6 files changed, 92 insertions(+), 7 deletions(-) create mode 100644 internal/provider/validator_channel_test.go diff --git a/Makefile b/Makefile index 6c745207..0b0cd02f 100644 --- a/Makefile +++ b/Makefile @@ -67,7 +67,9 @@ juju-unit-test: .PHONY: envtestlxd envtestlxd: ## envtestlxd: Under development - Include env var and run unit tests against lxd - JUJU_CONTROLLER_ADDRESSES=${CONTROLLER_ADDRESSES} JUJU_USERNAME=${USERNAME} JUJU_PASSWORD=${PASSWORD} JUJU_CA_CERT=${CA_CERT} TF_ACC=1 TEST_CLOUD=lxd go test ./... -v $(TESTARGS) -timeout 120m + JUJU_CONTROLLER_ADDRESSES=${CONTROLLER_ADDRESSES} \ + JUJU_USERNAME=${USERNAME} JUJU_PASSWORD=${PASSWORD} \ + JUJU_CA_CERT=${CA_CERT} TF_ACC=1 TEST_CLOUD=lxd go test ./... -v $(TESTARGS) -timeout 120m .PHONY: testlxd testlxd: diff --git a/go.mod b/go.mod index 859603d6..58241128 100644 --- a/go.mod +++ b/go.mod @@ -113,6 +113,7 @@ require ( github.com/json-iterator/go v1.1.12 // indirect github.com/juju/ansiterm v1.0.0 // indirect github.com/juju/blobstore/v3 v3.0.2 // indirect + github.com/juju/charm/v11 v11.1.0 // indirect github.com/juju/description/v5 v5.0.4 // indirect github.com/juju/featureflag v1.0.0 // indirect github.com/juju/gnuflag v1.0.0 // indirect diff --git a/go.sum b/go.sum index 2a12251d..719c75e4 100644 --- a/go.sum +++ b/go.sum @@ -324,6 +324,8 @@ github.com/juju/ansiterm v1.0.0 h1:gmMvnZRq7JZJx6jkfSq9/+2LMrVEwGwt7UR6G+lmDEg= github.com/juju/ansiterm v1.0.0/go.mod h1:PyXUpnI3olx3bsPcHt98FGPX/KCFZ1Fi+hw1XLI6384= github.com/juju/blobstore/v3 v3.0.2 h1:roZ4YBuZYmWId6y/6ZLQSAMmNlHOclHD8PQAMOQer6E= github.com/juju/blobstore/v3 v3.0.2/go.mod h1:NXEgMhrVH5744/zLfSkzsySlDQUpCgzvcNxjJJhICko= +github.com/juju/charm/v11 v11.1.0 h1:YTvFRugIhRMAe4z6Vr7Acw9oKnJBNfpwN9yTOqJv3r0= +github.com/juju/charm/v11 v11.1.0/go.mod h1:Mge5Ko3pPgocmk4v1pQgmBhF8BuBLGTCFu3jq83JvHk= github.com/juju/charm/v12 v12.0.0 h1:/h3YRMqbgxT89QkQGgMS/myOxuHy/kzBLCDOvodsoFY= github.com/juju/charm/v12 v12.0.0/go.mod h1:rX3no84EHT+qN+BGtwqPyvueC1Sxr0bXWxsbUd6i1iY= github.com/juju/clock v1.0.3 h1:yJHIsWXeU8j3QcBdiess09SzfiXRRrsjKPn2whnMeds= diff --git a/internal/provider/resource_application.go b/internal/provider/resource_application.go index c4151d6c..89703d6c 100644 --- a/internal/provider/resource_application.go +++ b/internal/provider/resource_application.go @@ -240,7 +240,7 @@ func (r *applicationResource) Schema(_ context.Context, _ resource.SchemaRequest stringplanmodifier.UseStateForUnknown(), }, Validators: []validator.String{ - stringIsChannelValidator{}, + StringIsChannelValidator{}, }, }, "revision": schema.Int64Attribute{ diff --git a/internal/provider/validator_channel.go b/internal/provider/validator_channel.go index 53eb063e..b747b9a8 100644 --- a/internal/provider/validator_channel.go +++ b/internal/provider/validator_channel.go @@ -10,26 +10,33 @@ import ( "github.com/juju/charm/v11" ) -type stringIsChannelValidator struct{} +type StringIsChannelValidator struct{} // Description returns a plain text description of the validator's behavior, suitable for a practitioner to understand its impact. -func (v stringIsChannelValidator) Description(context.Context) string { +func (v StringIsChannelValidator) Description(context.Context) string { return "string must conform to track/risk or track/risk/branch e.g. latest/stable" } // MarkdownDescription returns a markdown formatted description of the validator's behavior, suitable for a practitioner to understand its impact. -func (v stringIsChannelValidator) MarkdownDescription(context.Context) string { +func (v StringIsChannelValidator) MarkdownDescription(context.Context) string { return "string must conform to track/risk or track/risk/branch e.g. latest/stable" } // Validate runs the main validation logic of the validator, reading configuration data out of `req` and updating `resp` with diagnostics. -func (v stringIsChannelValidator) ValidateString(_ context.Context, req validator.StringRequest, resp *validator.StringResponse) { +func (v StringIsChannelValidator) ValidateString(_ context.Context, req validator.StringRequest, resp *validator.StringResponse) { // If the value is unknown or null, there is nothing to validate. if req.ConfigValue.IsUnknown() || req.ConfigValue.IsNull() { return } - if channel, err := charm.ParseChannel(req.ConfigValue.ValueString()); err != nil || channel.Track == "" || channel.Risk == "" { + if channel, err := charm.ParseChannel(req.ConfigValue.ValueString()); err != nil { + resp.Diagnostics.AddAttributeError( + req.Path, + "Invalid Channel", + err.Error(), + ) + return + } else if channel.Track == "" || channel.Risk == "" { resp.Diagnostics.AddAttributeError( req.Path, "Invalid Channel", diff --git a/internal/provider/validator_channel_test.go b/internal/provider/validator_channel_test.go new file mode 100644 index 00000000..980bc75e --- /dev/null +++ b/internal/provider/validator_channel_test.go @@ -0,0 +1,73 @@ +// Copyright 2024 Canonical Ltd. +// Licensed under the AGPLv3, see LICENCE file for details. + +package provider_test + +import ( + "context" + "testing" + + "github.com/hashicorp/terraform-plugin-framework/schema/validator" + "github.com/hashicorp/terraform-plugin-framework/types" + "github.com/juju/terraform-provider-juju/internal/provider" +) + +func TestChannelValidatorValid(t *testing.T) { + validChannels := []types.String{ + types.StringValue("track/stable"), + types.StringValue("track/edge/branch"), + types.StringNull(), + types.StringUnknown(), + } + + channelValidator := provider.StringIsChannelValidator{} + for _, channel := range validChannels { + req := validator.StringRequest{ + ConfigValue: channel, + } + var resp validator.StringResponse + channelValidator.ValidateString(context.Background(), req, &resp) + + if resp.Diagnostics.HasError() { + t.Errorf("errors %v", resp.Diagnostics.Errors()) + } + } +} + +func TestChannelValidatorInvalid(t *testing.T) { + invalidChannels := []struct { + str types.String + err string + }{{ + str: types.StringValue("track"), + err: "String must conform to track/risk or track/risk/branch, e.g. latest/stable", + }, { + str: types.StringValue("edge"), + err: "String must conform to track/risk or track/risk/branch, e.g. latest/stable", + }, { + str: types.StringValue(`track\risk`), + err: "String must conform to track/risk or track/risk/branch, e.g. latest/stable", + }, { + str: types.StringValue(`track/invalidrisk`), + err: `risk in channel "track/invalidrisk" not valid`, + }, { + str: types.StringValue(`track/invalidrisk/branch`), + err: `risk in channel "track/invalidrisk/branch" not valid`, + }} + + channelValidator := provider.StringIsChannelValidator{} + for _, test := range invalidChannels { + req := validator.StringRequest{ + ConfigValue: test.str, + } + var resp validator.StringResponse + channelValidator.ValidateString(context.Background(), req, &resp) + + if c := resp.Diagnostics.ErrorsCount(); c != 1 { + t.Errorf("expected one error, got %d", c) + } + if deets := resp.Diagnostics.Errors()[0].Detail(); deets != test.err { + t.Errorf("expected error %q, got %q", test.err, deets) + } + } +}