From f1210c58f132b8883837e197c74906da675b228d Mon Sep 17 00:00:00 2001 From: Kian Parvin Date: Tue, 6 Aug 2024 12:29:01 +0200 Subject: [PATCH 1/3] feat: add jaas validators The AvoidJAASValidator can be used to ensure a resource is not used with JAAS. E.g. juju_access_model should not be used with JAAS as there is a JAAS specific type for that. The RequiresJAASValidator can be used to ensure a resource is only used with JAAS. E.g. new JAAS specific resources should not be used when communicating with a Juju controller. --- internal/juju/client.go | 37 ++++++++++++ internal/provider/validator_avoid_jaas.go | 65 +++++++++++++++++++++ internal/provider/validator_require_jaas.go | 57 ++++++++++++++++++ 3 files changed, 159 insertions(+) create mode 100644 internal/provider/validator_avoid_jaas.go create mode 100644 internal/provider/validator_require_jaas.go diff --git a/internal/juju/client.go b/internal/juju/client.go index 807559eb..05e70d09 100644 --- a/internal/juju/client.go +++ b/internal/juju/client.go @@ -49,6 +49,14 @@ type Client struct { SSHKeys sshKeysClient Users usersClient Secrets secretsClient + + isJAAS func() bool +} + +// IsJAAS returns a boolean to indicate whether the controller configured is a JAAS controller. +// JAAS controllers offer additional functionality for permission management. +func (c Client) IsJAAS() bool { + return c.isJAAS() } type jujuModel struct { @@ -82,6 +90,12 @@ func NewClient(ctx context.Context, config ControllerConfiguration) (*Client, er modelUUIDcache: make(map[string]jujuModel), subCtx: tflog.NewSubsystem(ctx, LogJujuClient), } + // Client ID and secret are only set when connecting to JAAS. Use this as a fallback + // value if connecting to the controller fails. + defaultJAASCheck := false + if config.ClientID != "" && config.ClientSecret != "" { + defaultJAASCheck = true + } return &Client{ Applications: *newApplicationClient(sc), @@ -93,9 +107,32 @@ func NewClient(ctx context.Context, config ControllerConfiguration) (*Client, er SSHKeys: *newSSHKeysClient(sc), Users: *newUsersClient(sc), Secrets: *newSecretsClient(sc), + isJAAS: func() bool { return sc.IsJAAS(defaultJAASCheck) }, }, nil } +var checkJAASOnce sync.Once +var isJAAS bool + +// IsJAAS checks if the controller is a JAAS controller. +// It does this by checking whether it offers the "JIMM" facade which +// will only ever be offered by JAAS. The method accepts a default value +// and doesn't return an error because callers are not expected to fail if +// they can't determine whether they are connecting to JAAS. +// +// IsJAAS uses a synchronisation object to only perform the check once and return the same result. +func (sc *sharedClient) IsJAAS(defaultVal bool) bool { + checkJAASOnce.Do(func() { + conn, err := sc.GetConnection(nil) + if err != nil { + isJAAS = defaultVal + return + } + isJAAS = conn.BestFacadeVersion("JIMM") != 0 + }) + return isJAAS +} + // GetConnection returns a juju connection for use creating juju // api clients given the provided model name. func (sc *sharedClient) GetConnection(modelName *string) (api.Connection, error) { diff --git a/internal/provider/validator_avoid_jaas.go b/internal/provider/validator_avoid_jaas.go new file mode 100644 index 00000000..2492f287 --- /dev/null +++ b/internal/provider/validator_avoid_jaas.go @@ -0,0 +1,65 @@ +// Copyright 2024 Canonical Ltd. +// Licensed under the Apache License, Version 2.0, see LICENCE file for details. + +package provider + +import ( + "context" + + "github.com/hashicorp/terraform-plugin-framework/datasource" + "github.com/hashicorp/terraform-plugin-framework/diag" + "github.com/hashicorp/terraform-plugin-framework/provider" + "github.com/hashicorp/terraform-plugin-framework/resource" + "github.com/hashicorp/terraform-plugin-framework/tfsdk" + "github.com/juju/terraform-provider-juju/internal/juju" +) + +var _ datasource.ConfigValidator = &AvoidJAASValidator{} +var _ provider.ConfigValidator = &AvoidJAASValidator{} +var _ resource.ConfigValidator = &AvoidJAASValidator{} + +// AvoidJAASValidator enforces that the resource is not used with JAAS. +// Useful to direct users to more capable resources. +type AvoidJAASValidator struct { + Client *juju.Client + PreferredObject string +} + +func (v AvoidJAASValidator) Description(ctx context.Context) string { + return v.MarkdownDescription(ctx) +} + +func (v AvoidJAASValidator) MarkdownDescription(_ context.Context) string { + return "Enforces that this resource should not be used with JAAS" +} + +func (v AvoidJAASValidator) ValidateDataSource(ctx context.Context, req datasource.ValidateConfigRequest, resp *datasource.ValidateConfigResponse) { + resp.Diagnostics = v.Validate(ctx, req.Config) +} + +func (v AvoidJAASValidator) ValidateProvider(ctx context.Context, req provider.ValidateConfigRequest, resp *provider.ValidateConfigResponse) { + resp.Diagnostics = v.Validate(ctx, req.Config) +} + +func (v AvoidJAASValidator) ValidateResource(ctx context.Context, req resource.ValidateConfigRequest, resp *resource.ValidateConfigResponse) { + resp.Diagnostics = v.Validate(ctx, req.Config) +} + +func (v AvoidJAASValidator) Validate(ctx context.Context, config tfsdk.Config) diag.Diagnostics { + var diags diag.Diagnostics + + if v.Client != nil { + if v.Client.IsJAAS() { + hint := "" + if v.PreferredObject != "" { + hint = "Try the " + v.PreferredObject + " resource instead." + } + diags.AddWarning("Invalid use of resource with JAAS.", + "It is not supported to use this resource with a JAAS setup. "+ + hint+ + "JAAS offers additional enterprise features through the use of dedicated resources. "+ + "See the provider documentation for more details.") + } + } + return diags +} diff --git a/internal/provider/validator_require_jaas.go b/internal/provider/validator_require_jaas.go new file mode 100644 index 00000000..25af7ee9 --- /dev/null +++ b/internal/provider/validator_require_jaas.go @@ -0,0 +1,57 @@ +// Copyright 2024 Canonical Ltd. +// Licensed under the Apache License, Version 2.0, see LICENCE file for details. + +package provider + +import ( + "context" + + "github.com/hashicorp/terraform-plugin-framework/datasource" + "github.com/hashicorp/terraform-plugin-framework/diag" + "github.com/hashicorp/terraform-plugin-framework/provider" + "github.com/hashicorp/terraform-plugin-framework/resource" + "github.com/hashicorp/terraform-plugin-framework/tfsdk" + "github.com/juju/terraform-provider-juju/internal/juju" +) + +var _ datasource.ConfigValidator = &RequiresJAASValidator{} +var _ provider.ConfigValidator = &RequiresJAASValidator{} +var _ resource.ConfigValidator = &RequiresJAASValidator{} + +// RequiresJAASValidator enforces that the resource can only be used with JAAS. +type RequiresJAASValidator struct { + Client *juju.Client +} + +func (v RequiresJAASValidator) Description(ctx context.Context) string { + return v.MarkdownDescription(ctx) +} + +func (v RequiresJAASValidator) MarkdownDescription(_ context.Context) string { + return "Enforces that this resource can only be used with JAAS" +} + +func (v RequiresJAASValidator) ValidateDataSource(ctx context.Context, req datasource.ValidateConfigRequest, resp *datasource.ValidateConfigResponse) { + resp.Diagnostics = v.Validate(ctx, req.Config) +} + +func (v RequiresJAASValidator) ValidateProvider(ctx context.Context, req provider.ValidateConfigRequest, resp *provider.ValidateConfigResponse) { + resp.Diagnostics = v.Validate(ctx, req.Config) +} + +func (v RequiresJAASValidator) ValidateResource(ctx context.Context, req resource.ValidateConfigRequest, resp *resource.ValidateConfigResponse) { + resp.Diagnostics = v.Validate(ctx, req.Config) +} + +func (v RequiresJAASValidator) Validate(ctx context.Context, config tfsdk.Config) diag.Diagnostics { + var diags diag.Diagnostics + + if v.Client != nil { + if v.Client.IsJAAS() { + diags.AddError("Attempted use of resource without JAAS.", + "This resource can only be used with a JAAS setup offering additional enterprise features - see https://jaas.ai/ for more details.") + } + } + + return diags +} From 0be5485df6c9905c83c15bd56de28c6f650c27f4 Mon Sep 17 00:00:00 2001 From: Kian Parvin Date: Tue, 6 Aug 2024 15:10:31 +0200 Subject: [PATCH 2/3] feat: added missing godocs --- internal/provider/validator_avoid_jaas.go | 31 ++++++++++----------- internal/provider/validator_require_jaas.go | 19 ++++++------- 2 files changed, 22 insertions(+), 28 deletions(-) diff --git a/internal/provider/validator_avoid_jaas.go b/internal/provider/validator_avoid_jaas.go index 2492f287..01ec4843 100644 --- a/internal/provider/validator_avoid_jaas.go +++ b/internal/provider/validator_avoid_jaas.go @@ -8,14 +8,12 @@ import ( "github.com/hashicorp/terraform-plugin-framework/datasource" "github.com/hashicorp/terraform-plugin-framework/diag" - "github.com/hashicorp/terraform-plugin-framework/provider" "github.com/hashicorp/terraform-plugin-framework/resource" "github.com/hashicorp/terraform-plugin-framework/tfsdk" "github.com/juju/terraform-provider-juju/internal/juju" ) var _ datasource.ConfigValidator = &AvoidJAASValidator{} -var _ provider.ConfigValidator = &AvoidJAASValidator{} var _ resource.ConfigValidator = &AvoidJAASValidator{} // AvoidJAASValidator enforces that the resource is not used with JAAS. @@ -25,41 +23,40 @@ type AvoidJAASValidator struct { PreferredObject string } +// Description returns a plain text description of the validator's behavior, suitable for a practitioner to understand its impact. func (v AvoidJAASValidator) Description(ctx context.Context) string { return v.MarkdownDescription(ctx) } +// MarkdownDescription returns a markdown formatted description of the validator's behavior, suitable for a practitioner to understand its impact. func (v AvoidJAASValidator) MarkdownDescription(_ context.Context) string { return "Enforces that this resource should not be used with JAAS" } +// ValidateResource performs the validation on the data source. func (v AvoidJAASValidator) ValidateDataSource(ctx context.Context, req datasource.ValidateConfigRequest, resp *datasource.ValidateConfigResponse) { resp.Diagnostics = v.Validate(ctx, req.Config) } -func (v AvoidJAASValidator) ValidateProvider(ctx context.Context, req provider.ValidateConfigRequest, resp *provider.ValidateConfigResponse) { - resp.Diagnostics = v.Validate(ctx, req.Config) -} - +// ValidateResource performs the validation on the resource. func (v AvoidJAASValidator) ValidateResource(ctx context.Context, req resource.ValidateConfigRequest, resp *resource.ValidateConfigResponse) { resp.Diagnostics = v.Validate(ctx, req.Config) } +// Validate runs the main validation logic of the validator, reading configuration data out of `config` and returning with diagnostics. func (v AvoidJAASValidator) Validate(ctx context.Context, config tfsdk.Config) diag.Diagnostics { var diags diag.Diagnostics - if v.Client != nil { - if v.Client.IsJAAS() { - hint := "" - if v.PreferredObject != "" { - hint = "Try the " + v.PreferredObject + " resource instead." - } - diags.AddWarning("Invalid use of resource with JAAS.", - "It is not supported to use this resource with a JAAS setup. "+ - hint+ - "JAAS offers additional enterprise features through the use of dedicated resources. "+ - "See the provider documentation for more details.") + if v.Client != nil && v.Client.IsJAAS() { + hint := "" + if v.PreferredObject != "" { + hint = "Try the " + v.PreferredObject + " resource instead." } + diags.AddError("Invalid use of resource with JAAS.", + "It is not supported to use this resource with a JAAS setup. "+ + hint+ + "JAAS offers additional enterprise features through the use of dedicated resources. "+ + "See the provider documentation for more details.") } return diags } diff --git a/internal/provider/validator_require_jaas.go b/internal/provider/validator_require_jaas.go index 25af7ee9..6fcf238f 100644 --- a/internal/provider/validator_require_jaas.go +++ b/internal/provider/validator_require_jaas.go @@ -8,14 +8,12 @@ import ( "github.com/hashicorp/terraform-plugin-framework/datasource" "github.com/hashicorp/terraform-plugin-framework/diag" - "github.com/hashicorp/terraform-plugin-framework/provider" "github.com/hashicorp/terraform-plugin-framework/resource" "github.com/hashicorp/terraform-plugin-framework/tfsdk" "github.com/juju/terraform-provider-juju/internal/juju" ) var _ datasource.ConfigValidator = &RequiresJAASValidator{} -var _ provider.ConfigValidator = &RequiresJAASValidator{} var _ resource.ConfigValidator = &RequiresJAASValidator{} // RequiresJAASValidator enforces that the resource can only be used with JAAS. @@ -23,34 +21,33 @@ type RequiresJAASValidator struct { Client *juju.Client } +// Description returns a plain text description of the validator's behavior, suitable for a practitioner to understand its impact. func (v RequiresJAASValidator) Description(ctx context.Context) string { return v.MarkdownDescription(ctx) } +// // MarkdownDescription returns a markdown formatted description of the validator's behavior, suitable for a practitioner to understand its impact. func (v RequiresJAASValidator) MarkdownDescription(_ context.Context) string { return "Enforces that this resource can only be used with JAAS" } +// ValidateResource performs the validation on the data source. func (v RequiresJAASValidator) ValidateDataSource(ctx context.Context, req datasource.ValidateConfigRequest, resp *datasource.ValidateConfigResponse) { resp.Diagnostics = v.Validate(ctx, req.Config) } -func (v RequiresJAASValidator) ValidateProvider(ctx context.Context, req provider.ValidateConfigRequest, resp *provider.ValidateConfigResponse) { - resp.Diagnostics = v.Validate(ctx, req.Config) -} - +// ValidateResource performs the validation on the resource. func (v RequiresJAASValidator) ValidateResource(ctx context.Context, req resource.ValidateConfigRequest, resp *resource.ValidateConfigResponse) { resp.Diagnostics = v.Validate(ctx, req.Config) } +// Validate runs the main validation logic of the validator, reading configuration data out of `config` and returning with diagnostics. func (v RequiresJAASValidator) Validate(ctx context.Context, config tfsdk.Config) diag.Diagnostics { var diags diag.Diagnostics - if v.Client != nil { - if v.Client.IsJAAS() { - diags.AddError("Attempted use of resource without JAAS.", - "This resource can only be used with a JAAS setup offering additional enterprise features - see https://jaas.ai/ for more details.") - } + if v.Client != nil && v.Client.IsJAAS() { + diags.AddError("Attempted use of resource without JAAS.", + "This resource can only be used with a JAAS setup offering additional enterprise features - see https://jaas.ai/ for more details.") } return diags From e1d009757191780111aa1b290200ebb83345f206 Mon Sep 17 00:00:00 2001 From: Kian Parvin Date: Wed, 7 Aug 2024 11:22:53 +0200 Subject: [PATCH 3/3] feat: pr comments - avoid leaking connection - reword error messages - make validate function not exported --- internal/juju/client.go | 1 + internal/provider/validator_avoid_jaas.go | 16 +++++++++------- internal/provider/validator_require_jaas.go | 18 ++++++++++-------- 3 files changed, 20 insertions(+), 15 deletions(-) diff --git a/internal/juju/client.go b/internal/juju/client.go index 05e70d09..41d2d19a 100644 --- a/internal/juju/client.go +++ b/internal/juju/client.go @@ -128,6 +128,7 @@ func (sc *sharedClient) IsJAAS(defaultVal bool) bool { isJAAS = defaultVal return } + defer conn.Close() isJAAS = conn.BestFacadeVersion("JIMM") != 0 }) return isJAAS diff --git a/internal/provider/validator_avoid_jaas.go b/internal/provider/validator_avoid_jaas.go index 01ec4843..2e18e167 100644 --- a/internal/provider/validator_avoid_jaas.go +++ b/internal/provider/validator_avoid_jaas.go @@ -9,7 +9,7 @@ import ( "github.com/hashicorp/terraform-plugin-framework/datasource" "github.com/hashicorp/terraform-plugin-framework/diag" "github.com/hashicorp/terraform-plugin-framework/resource" - "github.com/hashicorp/terraform-plugin-framework/tfsdk" + "github.com/juju/terraform-provider-juju/internal/juju" ) @@ -35,28 +35,30 @@ func (v AvoidJAASValidator) MarkdownDescription(_ context.Context) string { // ValidateResource performs the validation on the data source. func (v AvoidJAASValidator) ValidateDataSource(ctx context.Context, req datasource.ValidateConfigRequest, resp *datasource.ValidateConfigResponse) { - resp.Diagnostics = v.Validate(ctx, req.Config) + resp.Diagnostics = v.validate() } // ValidateResource performs the validation on the resource. func (v AvoidJAASValidator) ValidateResource(ctx context.Context, req resource.ValidateConfigRequest, resp *resource.ValidateConfigResponse) { - resp.Diagnostics = v.Validate(ctx, req.Config) + resp.Diagnostics = v.validate() } -// Validate runs the main validation logic of the validator, reading configuration data out of `config` and returning with diagnostics. -func (v AvoidJAASValidator) Validate(ctx context.Context, config tfsdk.Config) diag.Diagnostics { +// validate runs the main validation logic of the validator, reading configuration data out of `config` and returning with diagnostics. +func (v AvoidJAASValidator) validate() diag.Diagnostics { var diags diag.Diagnostics + // Return without error if a nil client is detected. + // This is possible since validation is called at various points throughout resource creation. if v.Client != nil && v.Client.IsJAAS() { hint := "" if v.PreferredObject != "" { hint = "Try the " + v.PreferredObject + " resource instead." } diags.AddError("Invalid use of resource with JAAS.", - "It is not supported to use this resource with a JAAS setup. "+ + "This resource is not supported with JAAS. "+ hint+ "JAAS offers additional enterprise features through the use of dedicated resources. "+ - "See the provider documentation for more details.") + "See https://jaas.ai/ for more details.") } return diags } diff --git a/internal/provider/validator_require_jaas.go b/internal/provider/validator_require_jaas.go index 6fcf238f..7497dbbe 100644 --- a/internal/provider/validator_require_jaas.go +++ b/internal/provider/validator_require_jaas.go @@ -9,7 +9,7 @@ import ( "github.com/hashicorp/terraform-plugin-framework/datasource" "github.com/hashicorp/terraform-plugin-framework/diag" "github.com/hashicorp/terraform-plugin-framework/resource" - "github.com/hashicorp/terraform-plugin-framework/tfsdk" + "github.com/juju/terraform-provider-juju/internal/juju" ) @@ -26,28 +26,30 @@ func (v RequiresJAASValidator) Description(ctx context.Context) string { return v.MarkdownDescription(ctx) } -// // MarkdownDescription returns a markdown formatted description of the validator's behavior, suitable for a practitioner to understand its impact. +// MarkdownDescription returns a markdown formatted description of the validator's behavior, suitable for a practitioner to understand its impact. func (v RequiresJAASValidator) MarkdownDescription(_ context.Context) string { return "Enforces that this resource can only be used with JAAS" } // ValidateResource performs the validation on the data source. func (v RequiresJAASValidator) ValidateDataSource(ctx context.Context, req datasource.ValidateConfigRequest, resp *datasource.ValidateConfigResponse) { - resp.Diagnostics = v.Validate(ctx, req.Config) + resp.Diagnostics = v.validate() } // ValidateResource performs the validation on the resource. func (v RequiresJAASValidator) ValidateResource(ctx context.Context, req resource.ValidateConfigRequest, resp *resource.ValidateConfigResponse) { - resp.Diagnostics = v.Validate(ctx, req.Config) + resp.Diagnostics = v.validate() } -// Validate runs the main validation logic of the validator, reading configuration data out of `config` and returning with diagnostics. -func (v RequiresJAASValidator) Validate(ctx context.Context, config tfsdk.Config) diag.Diagnostics { +// validate runs the main validation logic of the validator, reading configuration data out of `config` and returning with diagnostics. +func (v RequiresJAASValidator) validate() diag.Diagnostics { var diags diag.Diagnostics - if v.Client != nil && v.Client.IsJAAS() { + // Return without error if a nil client is detected. + // This is possible since validation is called at various points throughout resource creation. + if v.Client != nil && !v.Client.IsJAAS() { diags.AddError("Attempted use of resource without JAAS.", - "This resource can only be used with a JAAS setup offering additional enterprise features - see https://jaas.ai/ for more details.") + "This resource can only be used with JAAS, which offers additional enterprise features - see https://jaas.ai/ for more details.") } return diags