From 73ee6407a9eefb561effa6a29fb31b6c55b62fd6 Mon Sep 17 00:00:00 2001 From: Alex Hung Date: Mon, 2 Oct 2023 15:43:35 -0700 Subject: [PATCH 1/6] Migrate artifactory_distribution_public_key to terraform-plugin-framework --- pkg/artifactory/provider/framework.go | 1 + pkg/artifactory/provider/resources.go | 1 - ...rce_artifactory_distribution_public_key.go | 502 ++++++++++++++---- ...rtifactory_distribution_public_key_test.go | 144 +++-- 4 files changed, 487 insertions(+), 161 deletions(-) diff --git a/pkg/artifactory/provider/framework.go b/pkg/artifactory/provider/framework.go index 24327273..d1a6821d 100644 --- a/pkg/artifactory/provider/framework.go +++ b/pkg/artifactory/provider/framework.go @@ -172,6 +172,7 @@ func (p *ArtifactoryProvider) Resources(ctx context.Context) []func() resource.R security.NewScopedTokenResource, security.NewPermissionTargetResource, security.NewGlobalEnvironmentResource, + security.NewDistributionPublicKeyResource, configuration.NewLdapSettingResource, configuration.NewLdapGroupSettingResource, configuration.NewBackupResource, diff --git a/pkg/artifactory/provider/resources.go b/pkg/artifactory/provider/resources.go index b46c50e8..888215b4 100644 --- a/pkg/artifactory/provider/resources.go +++ b/pkg/artifactory/provider/resources.go @@ -77,7 +77,6 @@ func resourcesMap() map[string]*schema.Resource { "artifactory_certificate": security.ResourceArtifactoryCertificate(), "artifactory_api_key": security.ResourceArtifactoryApiKey(), "artifactory_access_token": security.ResourceArtifactoryAccessToken(), - "artifactory_distribution_public_key": security.ResourceArtifactoryDistributionPublicKey(), "artifactory_general_security": configuration.ResourceArtifactoryGeneralSecurity(), "artifactory_oauth_settings": configuration.ResourceArtifactoryOauthSettings(), "artifactory_saml_settings": configuration.ResourceArtifactorySamlSettings(), diff --git a/pkg/artifactory/resource/security/resource_artifactory_distribution_public_key.go b/pkg/artifactory/resource/security/resource_artifactory_distribution_public_key.go index f2aea6a1..5cc7b337 100644 --- a/pkg/artifactory/resource/security/resource_artifactory_distribution_public_key.go +++ b/pkg/artifactory/resource/security/resource_artifactory_distribution_public_key.go @@ -2,150 +2,440 @@ package security import ( "context" + "crypto/rsa" + "crypto/x509" + "encoding/pem" "fmt" + "net/http" + "strings" - "github.com/hashicorp/terraform-plugin-sdk/v2/diag" - "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" - "github.com/hashicorp/terraform-plugin-sdk/v2/helper/validation" - "github.com/jfrog/terraform-provider-shared/packer" - "github.com/jfrog/terraform-provider-shared/predicate" + "github.com/hashicorp/terraform-plugin-framework-validators/stringvalidator" + "github.com/hashicorp/terraform-plugin-framework/attr" + "github.com/hashicorp/terraform-plugin-framework/diag" + "github.com/hashicorp/terraform-plugin-framework/path" + "github.com/hashicorp/terraform-plugin-framework/resource" + "github.com/hashicorp/terraform-plugin-framework/resource/schema" + "github.com/hashicorp/terraform-plugin-framework/resource/schema/planmodifier" + "github.com/hashicorp/terraform-plugin-framework/resource/schema/stringplanmodifier" + "github.com/hashicorp/terraform-plugin-framework/schema/validator" + "github.com/hashicorp/terraform-plugin-framework/types" + "github.com/hashicorp/terraform-plugin-framework/types/basetypes" + "github.com/hashicorp/terraform-plugin-go/tftypes" + "github.com/hashicorp/terraform-plugin-log/tflog" + + utilfw "github.com/jfrog/terraform-provider-shared/util/fw" utilsdk "github.com/jfrog/terraform-provider-shared/util/sdk" ) const DistributionPublicKeysAPIEndPoint = "artifactory/api/security/keys/trusted" -type distributionPublicKeyPayLoad struct { - KeyID string `json:"kid"` +func NewDistributionPublicKeyResource() resource.Resource { + return &DistributionPublicKeyResource{} +} + +type DistributionPublicKeyResource struct { + ProviderData utilsdk.ProvderMetadata +} + +// DistributionPublicKeyResourceModel describes the Terraform resource data model to match the +// resource schema. +type DistributionPublicKeyResourceModel struct { + KeyId types.String `tfsdk:"key_id"` + Alias types.String `tfsdk:"alias"` + Fingerprint types.String `tfsdk:"fingerprint"` + PublicKey TablessPublicKeyValue `tfsdk:"public_key"` + IssuedOn types.String `tfsdk:"issued_on"` + IssuedBy types.String `tfsdk:"issued_by"` + ValidUntil types.String `tfsdk:"valid_until"` +} + +func (r *DistributionPublicKeyResourceModel) FromAPIModel(ctx context.Context, model *DistributionPublicKeyAPIModel) diag.Diagnostics { + r.KeyId = types.StringValue(model.KeyId) + r.Alias = types.StringValue(model.Alias) + r.Fingerprint = types.StringValue(model.Fingerprint) + r.PublicKey = tablessPublicKeyValue(model.PublicKey) + r.IssuedOn = types.StringValue(model.IssuedOn) + r.IssuedBy = types.StringValue(model.IssuedBy) + r.ValidUntil = types.StringValue(model.ValidUntil) + + return nil +} + +// DistributionPublicKeyAPIModel describes the API data model. +type DistributionPublicKeyAPIModel struct { + KeyId string `json:"kid,omitempty"` Alias string `json:"alias"` - Fingerprint string `json:"fingerprint"` + Fingerprint string `json:"fingerprint,omitempty"` PublicKey string `json:"key"` - IssuedOn string `json:"issued_on"` - IssuedBy string `json:"issued_by"` - ValidUntil string `json:"valid_until"` + IssuedOn string `json:"issued_on,omitempty"` + IssuedBy string `json:"issued_by,omitempty"` + ValidUntil string `json:"valid_until,omitempty"` } type DistributionPublicKeysList struct { - Keys []distributionPublicKeyPayLoad `json:"keys"` + Keys []DistributionPublicKeyAPIModel `json:"keys"` } -func ResourceArtifactoryDistributionPublicKey() *schema.Resource { +func (r *DistributionPublicKeyResource) Metadata(ctx context.Context, req resource.MetadataRequest, resp *resource.MetadataResponse) { + resp.TypeName = "artifactory_distribution_public_key" +} - type keyPost struct { - Alias string `json:"alias"` - PublicKey string `json:"public_key"` +func (r *DistributionPublicKeyResource) Schema(ctx context.Context, req resource.SchemaRequest, resp *resource.SchemaResponse) { + resp.Schema = schema.Schema{ + MarkdownDescription: "Manage the public GPG trusted keys used to verify distributed release bundles.", + Attributes: map[string]schema.Attribute{ + "key_id": schema.StringAttribute{ + MarkdownDescription: "Returns the key id by which this key is referenced in Artifactory.", + Computed: true, + PlanModifiers: []planmodifier.String{ + stringplanmodifier.UseStateForUnknown(), + }, + }, + "alias": schema.StringAttribute{ + MarkdownDescription: "Will be used as an identifier when uploading/retrieving the public key via REST API.", + Required: true, + Validators: []validator.String{stringvalidator.LengthAtLeast(1)}, + PlanModifiers: []planmodifier.String{ + stringplanmodifier.RequiresReplaceIfConfigured(), + }, + }, + "fingerprint": schema.StringAttribute{ + MarkdownDescription: "Returns the computed key fingerprint", + Computed: true, + PlanModifiers: []planmodifier.String{ + stringplanmodifier.UseStateForUnknown(), + }, + }, + "public_key": schema.StringAttribute{ + MarkdownDescription: "The Public key to add as a trusted distribution GPG key. To avoid state drift, ensure there are no leading tab or space characters for each line.", + Required: true, + CustomType: TablessPublicKeyType{}, + Validators: []validator.String{publicKeyMustBeGPGOrRSA()}, + PlanModifiers: []planmodifier.String{ + stringplanmodifier.RequiresReplaceIfConfigured(), + }, + }, + "issued_on": schema.StringAttribute{ + MarkdownDescription: "Returns the date/time when this GPG key was created.", + Computed: true, + PlanModifiers: []planmodifier.String{ + stringplanmodifier.UseStateForUnknown(), + }, + }, + "issued_by": schema.StringAttribute{ + MarkdownDescription: "Returns the name and eMail address of issuer.", + Computed: true, + PlanModifiers: []planmodifier.String{ + stringplanmodifier.UseStateForUnknown(), + }, + }, + "valid_until": schema.StringAttribute{ + MarkdownDescription: "Returns the date/time when this GPG key expires.", + Computed: true, + PlanModifiers: []planmodifier.String{ + stringplanmodifier.UseStateForUnknown(), + }, + }, + }, } +} - var distributionPublicKeySchema = map[string]*schema.Schema{ - "key_id": { - Type: schema.TypeString, - Computed: true, - Description: "Returns the key id by which this key is referenced in Artifactory.", - }, - "alias": { - Type: schema.TypeString, - Required: true, - Description: "Will be used as an identifier when uploading/retrieving the public key via REST API.", - ForceNew: true, - ValidateFunc: validation.StringIsNotEmpty, - }, - "fingerprint": { - Type: schema.TypeString, - Computed: true, - Description: "Returns the computed key fingerprint", - }, - "public_key": { - Type: schema.TypeString, - Required: true, - Description: "The Public key to add as a trusted distribution GPG key.", - ForceNew: true, - StateFunc: stripTabs, - ValidateDiagFunc: validatePublicKey, - }, - "issued_on": { - Type: schema.TypeString, - Computed: true, - Description: "Returns the date/time when this GPG key was created.", - }, - "issued_by": { - Type: schema.TypeString, - Computed: true, - Description: "Returns the name and eMail address of issuer.", - }, - "valid_until": { - Type: schema.TypeString, - Computed: true, - Description: "Returns the date/time when this GPG key expires.", - }, +func (r *DistributionPublicKeyResource) Configure(ctx context.Context, req resource.ConfigureRequest, resp *resource.ConfigureResponse) { + // Prevent panic if the provider has not been configured. + if req.ProviderData == nil { + return } + r.ProviderData = req.ProviderData.(utilsdk.ProvderMetadata) +} - var resultPacker = packer.Universal(predicate.SchemaHasKey(distributionPublicKeySchema)) +func (r *DistributionPublicKeyResource) Create(ctx context.Context, req resource.CreateRequest, resp *resource.CreateResponse) { + var plan *DistributionPublicKeyResourceModel + // Read Terraform plan data into the model + resp.Diagnostics.Append(req.Plan.Get(ctx, &plan)...) + if resp.Diagnostics.HasError() { + return + } - var resourceDistributionPublicKeyCreate = func(_ context.Context, d *schema.ResourceData, m interface{}) diag.Diagnostics { + // Convert from Terraform data model into API data model + var publicKey DistributionPublicKeyAPIModel - result := distributionPublicKeyPayLoad{} + body := DistributionPublicKeyAPIModel{ + Alias: plan.Alias.ValueString(), + PublicKey: stripTabs(plan.PublicKey.ValueString()), + } - resp, err := m.(utilsdk.ProvderMetadata).Client.R().SetBody(keyPost{ - d.Get("alias").(string), - stripTabs(d.Get("public_key").(string)), - }).SetResult(&result).Post(DistributionPublicKeysAPIEndPoint) - if err != nil { - return diag.FromErr(err) - } - if resp.IsError() { - return diag.FromErr(fmt.Errorf("unable to add key: http request failed: %s", resp.Status())) - } + response, err := r.ProviderData.Client.R(). + SetBody(body). + SetResult(&publicKey). + Post(DistributionPublicKeysAPIEndPoint) - d.SetId(result.KeyID) + if err != nil { + utilfw.UnableToCreateResourceError(resp, err.Error()) + return + } - return diag.FromErr(resultPacker(&result, d)) + // Return error if the HTTP status code is not 200 OK + if response.StatusCode() != http.StatusCreated { + utilfw.UnableToCreateResourceError(resp, response.String()) + return } - var resourceDistributionPublicKeyRead = func(_ context.Context, d *schema.ResourceData, m interface{}) diag.Diagnostics { + // Assign the resource ID for the resource in the state + resp.Diagnostics.Append(plan.FromAPIModel(ctx, &publicKey)...) + // data.KeyId = types.StringValue(publicKey.KeyId) - data := DistributionPublicKeysList{} - resp, err := m.(utilsdk.ProvderMetadata).Client.R().SetResult(&data).Get(DistributionPublicKeysAPIEndPoint) - if err != nil { - return diag.FromErr(err) - } - if resp.IsError() { - return diag.FromErr(fmt.Errorf("unable to read key: http request failed: %s", resp.Status())) + // Save data into Terraform state + resp.Diagnostics.Append(resp.State.Set(ctx, &plan)...) +} + +func (r *DistributionPublicKeyResource) Read(ctx context.Context, req resource.ReadRequest, resp *resource.ReadResponse) { + var state *DistributionPublicKeyResourceModel + // Read Terraform prior state data into the model + resp.Diagnostics.Append(req.State.Get(ctx, &state)...) + if resp.Diagnostics.HasError() { + return + } + + // Convert from Terraform data model into API data model + var publicKeys DistributionPublicKeysList + + response, err := r.ProviderData.Client.R(). + SetResult(&publicKeys). + Get(DistributionPublicKeysAPIEndPoint) + + // Treat HTTP 404 Not Found status as a signal to recreate resource + // and return early + if err != nil { + if response.StatusCode() == http.StatusNotFound { + resp.State.RemoveResource(ctx) } + utilfw.UnableToRefreshResourceError(resp, response.String()) + return + } - for _, key := range data.Keys { - if key.KeyID == d.Id() { - return diag.FromErr(resultPacker(&key, d)) - } + // Convert from the API data model to the Terraform data model + // and refresh any attribute values. + for _, key := range publicKeys.Keys { + if key.Alias == state.Alias.ValueString() { + resp.Diagnostics.Append(state.FromAPIModel(ctx, &key)...) + tflog.Debug(ctx, fmt.Sprintf("state after: %v", state)) } + } + if resp.Diagnostics.HasError() { + return + } + + // Save updated data into Terraform state + resp.Diagnostics.Append(resp.State.Set(ctx, &state)...) +} + +func (r *DistributionPublicKeyResource) Update(ctx context.Context, req resource.UpdateRequest, resp *resource.UpdateResponse) { + // Update is not supported +} + +func (r *DistributionPublicKeyResource) Delete(ctx context.Context, req resource.DeleteRequest, resp *resource.DeleteResponse) { + var state DistributionPublicKeyResourceModel + + // Read Terraform prior state data into the model + resp.Diagnostics.Append(req.State.Get(ctx, &state)...) - // If the ID is updated to blank, this tells Terraform the resource no longer exist - d.SetId("") - return nil + response, err := r.ProviderData.Client.R(). + Delete(fmt.Sprintf("%s/%s", DistributionPublicKeysAPIEndPoint, state.KeyId.ValueString())) + + if err != nil { + utilfw.UnableToDeleteResourceError(resp, response.String()) + return } - var resourceDistributionPublictedKeyDelete = func(_ context.Context, d *schema.ResourceData, m interface{}) diag.Diagnostics { - resp, err := m.(utilsdk.ProvderMetadata).Client.R().Delete(fmt.Sprintf("%s/%s", DistributionPublicKeysAPIEndPoint, d.Id())) - if err != nil { - return diag.FromErr(err) - } + // Return error if the HTTP status code is not 204 No Content or 404 Not Found + if response.StatusCode() != http.StatusNoContent && response.StatusCode() != http.StatusOK { + utilfw.UnableToDeleteResourceError(resp, response.String()) + return + } - if resp.IsError() { - return diag.FromErr(fmt.Errorf("unable to delete key: http request failed: %s", resp.Status())) - } + // If the logic reaches here, it implicitly succeeded and will remove + // the resource from state if there are no other errors. +} + +// ImportState imports the resource into the Terraform state. +func (r *DistributionPublicKeyResource) ImportState(ctx context.Context, req resource.ImportStateRequest, resp *resource.ImportStateResponse) { + resource.ImportStatePassthroughID(ctx, path.Root("alias"), req, resp) +} + +type publicKeyValidator struct{} + +func (v publicKeyValidator) Description(_ context.Context) string { + return "public key must be either PGP or RSA." +} + +func (v publicKeyValidator) MarkdownDescription(_ context.Context) string { + return "public key must be either PGP or RSA." +} + +func (v publicKeyValidator) 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 + } - d.SetId("") - return nil + stripped := strings.ReplaceAll(req.ConfigValue.ValueString(), "\t", "") + // currently can't validate GPG + if strings.Contains(stripped, "BEGIN PGP PUBLIC KEY BLOCK") { + resp.Diagnostics.AddAttributeWarning( + req.Path, + "Usage of GPG can't be validated.", + "Due to limitations of go libraries, your GPG key can't be validated client side.", + ) + return } - return &schema.Resource{ - CreateContext: resourceDistributionPublicKeyCreate, - DeleteContext: resourceDistributionPublictedKeyDelete, - ReadContext: resourceDistributionPublicKeyRead, + pubPem, _ := pem.Decode([]byte(stripped)) + if pubPem == nil { + resp.Diagnostics.AddAttributeError( + req.Path, + "RSA public key not in pem format.", + "RSA public key not in pem format.", + ) + return + } - Importer: &schema.ResourceImporter{ - StateContext: schema.ImportStatePassthroughContext, - }, - Description: "Manage the public GPG trusted keys used to verify distributed release bundles", + if !strings.Contains(pubPem.Type, "PUBLIC KEY") { + resp.Diagnostics.AddAttributeError( + req.Path, + "RSA public key is of the wrong type.", + fmt.Sprintf("RSA public keymust container the header 'PUBLIC KEY': Pem Type: %s ", pubPem.Type), + ) + return + } + + var parsedKey interface{} + if _, err := x509.ParsePKIXPublicKey(pubPem.Bytes); err != nil { + resp.Diagnostics.AddAttributeError( + req.Path, + "Unable to parse RSA public key.", + err.Error(), + ) + return + } + + if _, ok := parsedKey.(*rsa.PublicKey); !ok { + resp.Diagnostics.AddAttributeError( + req.Path, + "Unable to cast to RSA public key data type.", + "", + ) + return + } +} + +func publicKeyMustBeGPGOrRSA() publicKeyValidator { + return publicKeyValidator{} +} + +// Ensure the implementation satisfies the expected interfaces +var _ basetypes.StringTypable = TablessPublicKeyType{} + +type TablessPublicKeyType struct { + basetypes.StringType +} - Schema: distributionPublicKeySchema, +func (t TablessPublicKeyType) Equal(o attr.Type) bool { + other, ok := o.(TablessPublicKeyType) + + if !ok { + return false + } + + return t.StringType.Equal(other.StringType) +} + +func (t TablessPublicKeyType) String() string { + return "TablessPublicKeyType" +} + +func (t TablessPublicKeyType) ValueFromString(ctx context.Context, in basetypes.StringValue) (basetypes.StringValuable, diag.Diagnostics) { + // TablessPublicKeyValue defined in the value type section + value := TablessPublicKeyValue{ + StringValue: in, } + + return value, nil +} + +func (t TablessPublicKeyType) ValueFromTerraform(ctx context.Context, in tftypes.Value) (attr.Value, error) { + attrValue, err := t.StringType.ValueFromTerraform(ctx, in) + + if err != nil { + return nil, err + } + + stringValue, ok := attrValue.(basetypes.StringValue) + + if !ok { + return nil, fmt.Errorf("unexpected value type of %T", attrValue) + } + + stringValuable, diags := t.ValueFromString(ctx, stringValue) + + if diags.HasError() { + return nil, fmt.Errorf("unexpected error converting StringValue to StringValuable: %v", diags) + } + + return stringValuable, nil +} + +func (t TablessPublicKeyType) ValueType(ctx context.Context) attr.Value { + // CustomStringValue defined in the value type section + return TablessPublicKeyValue{} +} + +func tablessPublicKeyValue(value string) TablessPublicKeyValue { + return TablessPublicKeyValue{ + StringValue: basetypes.NewStringValue(value), + } +} + +// Ensure the implementation satisfies the expected interfaces +var _ basetypes.StringValuableWithSemanticEquals = TablessPublicKeyValue{} + +type TablessPublicKeyValue struct { + basetypes.StringValue +} + +func (v TablessPublicKeyValue) Equal(o attr.Value) bool { + other, ok := o.(TablessPublicKeyValue) + + if !ok { + return false + } + + return v.StringValue.Equal(other.StringValue) +} + +func (v TablessPublicKeyValue) Type(ctx context.Context) attr.Type { + // CustomStringType defined in the schema type section + return TablessPublicKeyType{} +} + +// StringSemanticEquals returns true if the given string value is semantically equal to the current string value. (case-insensitive) +func (v TablessPublicKeyValue) StringSemanticEquals(ctx context.Context, newValuable basetypes.StringValuable) (bool, diag.Diagnostics) { + var diags diag.Diagnostics + + newValue, ok := newValuable.(TablessPublicKeyValue) + if !ok { + diags.AddError( + "Semantic Equality Check Error", + "An unexpected value type was received while performing semantic equality checks. "+ + "Please report this to the provider developers.\n\n"+ + "Expected Value Type: "+fmt.Sprintf("%T", v)+"\n"+ + "Got Value Type: "+fmt.Sprintf("%T", newValuable), + ) + + return false, diags + } + + tflog.Debug(ctx, fmt.Sprintf("newValue.ValueString(): %s", newValue.ValueString())) + tflog.Debug(ctx, fmt.Sprintf("v.ValueString(): %s", v.ValueString())) + + return strings.EqualFold(stripTabs(newValue.ValueString()), v.ValueString()), diags } diff --git a/pkg/artifactory/resource/security/resource_artifactory_distribution_public_key_test.go b/pkg/artifactory/resource/security/resource_artifactory_distribution_public_key_test.go index 6fb4cb67..20690de1 100644 --- a/pkg/artifactory/resource/security/resource_artifactory_distribution_public_key_test.go +++ b/pkg/artifactory/resource/security/resource_artifactory_distribution_public_key_test.go @@ -15,7 +15,79 @@ import ( const resource_name = "artifactory_distribution_public_key" -func TestAccDistributionPublicKeyFormatCheck(t *testing.T) { +const template = `resource "%s" "%s" { + alias = "%s" + public_key = <"), + ), + ConfigPlanChecks: acctest.ConfigPlanChecks, + }, + { + ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories, + Config: keyBasic, + PlanOnly: true, + ConfigPlanChecks: acctest.ConfigPlanChecks, + }, + }, + }) +} + +func TestAccDistributionPublicKey_FormatCheck(t *testing.T) { id, _, name := testutil.MkNames("mykey", resource_name) keyBasic := fmt.Sprintf(` resource "%s" "%s" { @@ -24,77 +96,41 @@ func TestAccDistributionPublicKeyFormatCheck(t *testing.T) { } `, resource_name, name, id) resource.Test(t, resource.TestCase{ - PreCheck: func() { acctest.PreCheck(t) }, - ProviderFactories: acctest.ProviderFactories, + PreCheck: func() { acctest.PreCheck(t) }, + ProtoV5ProviderFactories: acctest.ProtoV5MuxProviderFactories, Steps: []resource.TestStep{ { Config: keyBasic, - ExpectError: regexp.MustCompile(".*rsa public key not in pem format.*"), + ExpectError: regexp.MustCompile(".*RSA public key not in pem format.*"), }, }, }) } -func TestAccDistributionPublicKeyCreate(t *testing.T) { - id, fqrn, name := testutil.MkNames("mykey", resource_name) - keyBasic := fmt.Sprintf(` - resource "%s" "%s" { - alias = "foo-alias%d" - public_key = <"), ), }, { - ResourceName: fqrn, - ImportState: true, - ImportStateVerify: true, + ResourceName: fqrn, + ImportState: true, + ImportStateId: name, + ImportStateVerify: true, + ImportStateVerifyIdentifierAttribute: "alias", }, }, }) @@ -119,7 +155,7 @@ func testAccCheckDistributionPublicKeyDestroy(id string) func(*terraform.State) } for _, key := range data.Keys { - if key.KeyID == rs.Primary.ID { + if key.KeyId == rs.Primary.ID { return fmt.Errorf("error: Distribution Public Key %s still exists", rs.Primary.ID) } } From c6e8568e38444c124e5108989cd763fe924d162a Mon Sep 17 00:00:00 2001 From: Alex Hung Date: Mon, 2 Oct 2023 15:55:17 -0700 Subject: [PATCH 2/6] Improve code readability Replace deprecated Golang call with new one. --- .../resource_artifactory_backup.go | 22 +++++++------------ .../resource_artifactory_certificate.go | 11 +++++----- .../resource_artifactory_certificate_test.go | 1 + 3 files changed, 14 insertions(+), 20 deletions(-) diff --git a/pkg/artifactory/resource/configuration/resource_artifactory_backup.go b/pkg/artifactory/resource/configuration/resource_artifactory_backup.go index dc0ebef9..1f957445 100644 --- a/pkg/artifactory/resource/configuration/resource_artifactory_backup.go +++ b/pkg/artifactory/resource/configuration/resource_artifactory_backup.go @@ -246,14 +246,14 @@ func (r *BackupResource) Create(ctx context.Context, req resource.CreateRequest, } func (r *BackupResource) Read(ctx context.Context, req resource.ReadRequest, resp *resource.ReadResponse) { - var data *BackupResourceModel + var state *BackupResourceModel // Read Terraform prior state data into the model - resp.Diagnostics.Append(req.State.Get(ctx, &data)...) + resp.Diagnostics.Append(req.State.Get(ctx, &state)...) if resp.Diagnostics.HasError() { return } - backups := Backups{} + var backups Backups _, err := r.ProviderData.Client.R(). SetResult(&backups). Get("artifactory/api/system/configuration") @@ -262,12 +262,12 @@ func (r *BackupResource) Read(ctx context.Context, req resource.ReadRequest, res return } - matchedBackup := FindConfigurationById[BackupAPIModel](backups.BackupArr, data.Key.ValueString()) + matchedBackup := FindConfigurationById[BackupAPIModel](backups.BackupArr, state.Key.ValueString()) if matchedBackup == nil { resp.Diagnostics.AddAttributeWarning( path.Root("key"), "no matching backup found", - data.Key.ValueString(), + state.Key.ValueString(), ) resp.State.RemoveResource(ctx) return @@ -275,13 +275,13 @@ func (r *BackupResource) Read(ctx context.Context, req resource.ReadRequest, res // Convert from the API data model to the Terraform data model // and refresh any attribute values. - resp.Diagnostics.Append(data.FromAPIModel(ctx, matchedBackup)...) + resp.Diagnostics.Append(state.FromAPIModel(ctx, matchedBackup)...) if resp.Diagnostics.HasError() { return } // Save updated data into Terraform state - resp.Diagnostics.Append(resp.State.Set(ctx, &data)...) + resp.Diagnostics.Append(resp.State.Set(ctx, &state)...) } func (r *BackupResource) Update(ctx context.Context, req resource.UpdateRequest, resp *resource.UpdateResponse) { @@ -340,16 +340,10 @@ func (r *BackupResource) Delete(ctx context.Context, req resource.DeleteRequest, return } - var backup BackupAPIModel - resp.Diagnostics.Append(data.toAPIModel(ctx, &backup)...) - if resp.Diagnostics.HasError() { - return - } - deleteBackupConfig := fmt.Sprintf(` backups: %s: ~ -`, backup.Key) +`, data.Key.ValueString()) err := SendConfigurationPatch([]byte(deleteBackupConfig), r.ProviderData) if err != nil { diff --git a/pkg/artifactory/resource/security/resource_artifactory_certificate.go b/pkg/artifactory/resource/security/resource_artifactory_certificate.go index c5baa3bc..9512c69f 100644 --- a/pkg/artifactory/resource/security/resource_artifactory_certificate.go +++ b/pkg/artifactory/resource/security/resource_artifactory_certificate.go @@ -7,7 +7,6 @@ import ( "encoding/hex" "encoding/pem" "fmt" - "io/ioutil" "os" "strings" @@ -71,7 +70,7 @@ func ResourceArtifactoryCertificate() *schema.Resource { if _, err := os.Stat(value.(string)); err != nil { return nil, append(errors, err) } - data, err := ioutil.ReadFile(value.(string)) + data, err := os.ReadFile(value.(string)) if err != nil { return nil, append(errors, err) } @@ -109,7 +108,7 @@ func ResourceArtifactoryCertificate() *schema.Resource { } func calculateFingerprint(_ context.Context, d *schema.ResourceDiff, _ interface{}) error { - content, err := getContentFromDiff(d) + content, _ := getContentFromDiff(d) fingerprint, err := calculateFingerPrint(content) if err != nil { return err @@ -203,7 +202,7 @@ func resourceCertificateRead(_ context.Context, d *schema.ResourceData, m interf setValue("issued_to", (*cert).IssuedTo) errors := setValue("valid_until", (*cert).ValidUntil) - if errors != nil && len(errors) > 0 { + if len(errors) > 0 { return diag.Errorf("failed to pack certificate %q", errors) } @@ -227,7 +226,7 @@ func getContentFromDiff(d *schema.ResourceDiff) (string, error) { return content.(string), nil } if fileExists { - data, err := ioutil.ReadFile(file.(string)) + data, err := os.ReadFile(file.(string)) if err != nil { return "", err } @@ -242,7 +241,7 @@ func getContentFromData(d *schema.ResourceData) (string, error) { return content.(string), nil } if file, ok := d.GetOk("file"); ok { - data, err := ioutil.ReadFile(file.(string)) + data, err := os.ReadFile(file.(string)) if err != nil { return "", err } diff --git a/pkg/artifactory/resource/security/resource_artifactory_certificate_test.go b/pkg/artifactory/resource/security/resource_artifactory_certificate_test.go index 0b6762b5..34a37aa4 100644 --- a/pkg/artifactory/resource/security/resource_artifactory_certificate_test.go +++ b/pkg/artifactory/resource/security/resource_artifactory_certificate_test.go @@ -33,6 +33,7 @@ func TestAccCertHasFileAndContentFails(t *testing.T) { }, }) } + func TestAccCertWithFileMissing(t *testing.T) { const certWithMissingFile = ` resource "artifactory_certificate" "fail" { From 34d06ca05408b6bb626fecafc36fdbfc19290800 Mon Sep 17 00:00:00 2001 From: Alex Hung Date: Mon, 2 Oct 2023 16:21:28 -0700 Subject: [PATCH 3/6] Update CHANGELOG --- CHANGELOG.md | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index a5e42555..737b3af1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,11 @@ +## 9.3.0 (Oct 3, 2023) + +IMPROVEMENTS: + +* resource/artifactory_distribution_public_key is migrated to Plugin Framework. PR: [#817](https://github.com/jfrog/terraform-provider-artifactory/pull/817) +* resource/artifactory_remote_\*\_repository: Fix incorrect default value for `store_artifacts_locally` attribute in documentation. PR: [#816](https://github.com/jfrog/terraform-provider-artifactory/pull/816) + + ## 9.2.1 (Sep 29, 2023). Tested on Artifactory 7.68.11 with Terraform CLI v1.5.7 IMPROVEMENTS: From 7d8caaf8ecf5a5ebe0a1861a49981ff28d328f68 Mon Sep 17 00:00:00 2001 From: JFrog CI Date: Tue, 3 Oct 2023 00:10:32 +0000 Subject: [PATCH 4/6] JFrog Pipelines - Add Artifactory version to CHANGELOG.md --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 737b3af1..81a26b86 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,4 +1,4 @@ -## 9.3.0 (Oct 3, 2023) +## 9.3.0 (Oct 3, 2023). Tested on Artifactory 7.68.13 with Terraform CLI v1.5.7 IMPROVEMENTS: From d38aa2fc644179c579862210e3a940e1b1cf6ae3 Mon Sep 17 00:00:00 2001 From: Alex Hung Date: Thu, 5 Oct 2023 14:05:23 -0700 Subject: [PATCH 5/6] Update TFproviderTest.yml Bump timeout to 90 min --- .jfrog-pipelines/TFproviderTest.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.jfrog-pipelines/TFproviderTest.yml b/.jfrog-pipelines/TFproviderTest.yml index 81c37606..653dd6a3 100644 --- a/.jfrog-pipelines/TFproviderTest.yml +++ b/.jfrog-pipelines/TFproviderTest.yml @@ -19,7 +19,7 @@ pipelines: configuration: #nodePool: default priority: 1 - timeoutSeconds: 3600 # 60 minutes + timeoutSeconds: 5400 # 90 minutes runtime: type: image image: @@ -230,4 +230,4 @@ pipelines: send_notification partnership_slack --text "${pipeline_name} failed on <${step_url}|${step_name}> step." fi onComplete: - - echo "Cleaning up" \ No newline at end of file + - echo "Cleaning up" From 79db6fab2251efd35306f3092ccd47c98bbbaacb Mon Sep 17 00:00:00 2001 From: Alex Hung Date: Thu, 5 Oct 2023 14:22:12 -0700 Subject: [PATCH 6/6] Update TFproviderTest.yml Revert Pipeline timeout bump --- .jfrog-pipelines/TFproviderTest.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.jfrog-pipelines/TFproviderTest.yml b/.jfrog-pipelines/TFproviderTest.yml index 653dd6a3..d261bf87 100644 --- a/.jfrog-pipelines/TFproviderTest.yml +++ b/.jfrog-pipelines/TFproviderTest.yml @@ -19,7 +19,7 @@ pipelines: configuration: #nodePool: default priority: 1 - timeoutSeconds: 5400 # 90 minutes + timeoutSeconds: 3600 # 60 minutes runtime: type: image image: