Skip to content

Commit

Permalink
feat: pr comments
Browse files Browse the repository at this point in the history
- avoid leaking connection
- reword error messages
- make validate function not exported
  • Loading branch information
kian99 committed Aug 7, 2024
1 parent 0be5485 commit e1d0097
Show file tree
Hide file tree
Showing 3 changed files with 20 additions and 15 deletions.
1 change: 1 addition & 0 deletions internal/juju/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,7 @@ func (sc *sharedClient) IsJAAS(defaultVal bool) bool {
isJAAS = defaultVal
return
}
defer conn.Close()
isJAAS = conn.BestFacadeVersion("JIMM") != 0
})
return isJAAS
Expand Down
16 changes: 9 additions & 7 deletions internal/provider/validator_avoid_jaas.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand All @@ -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
}
18 changes: 10 additions & 8 deletions internal/provider/validator_require_jaas.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand All @@ -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
Expand Down

0 comments on commit e1d0097

Please sign in to comment.