From ed80cb16240b66dcd9a20de7f27196f026cb228e Mon Sep 17 00:00:00 2001 From: Christian Bianchi Date: Tue, 20 Aug 2024 11:00:53 +0200 Subject: [PATCH] Remove private endpoints from service (#138) --- .github/workflows/e2e.yaml | 2 + Makefile | 2 +- docs/resources/service.md | 2 +- pkg/internal/api/errors.go | 13 ++++ pkg/resource/models/service_resource.go | 1 - pkg/resource/models/service_resource_test.go | 34 ---------- pkg/resource/private_endpoint_registration.go | 22 +++---- pkg/resource/service.go | 66 +++++-------------- pkg/resource/service_test.go | 18 ----- 9 files changed, 43 insertions(+), 117 deletions(-) create mode 100644 pkg/internal/api/errors.go diff --git a/.github/workflows/e2e.yaml b/.github/workflows/e2e.yaml index 4f2e691..b63dce5 100644 --- a/.github/workflows/e2e.yaml +++ b/.github/workflows/e2e.yaml @@ -1,6 +1,8 @@ name: E2E tests on: + workflow_dispatch: + inputs: {} schedule: - cron: "0 3 * * *" diff --git a/Makefile b/Makefile index 9da27f2..7256b2e 100644 --- a/Makefile +++ b/Makefile @@ -59,7 +59,7 @@ ifneq ($(shell test -f $(GOLANGCILINT) && echo -n yes),yes) GOLANGCILINT = /tmp/golangci-lint endif ensure-golangci-lint: ## Download golangci-lint locally if necessary. - $(call go-get-tool,$(GOLANGCILINT),github.com/golangci/golangci-lint/cmd/golangci-lint@v1.59.1) + $(call go-get-tool,$(GOLANGCILINT),github.com/golangci/golangci-lint/cmd/golangci-lint@v1.60.1) # go-get-tool will 'go get' any package $2 and install it to $1. define go-get-tool diff --git a/docs/resources/service.md b/docs/resources/service.md index 5f29498..35f4eda 100644 --- a/docs/resources/service.md +++ b/docs/resources/service.md @@ -58,7 +58,7 @@ resource "clickhouse_service" "service" { - `num_replicas` (Number) Number of replicas for the service. Available only for 'production' services. Must be between 3 and 20. Contact support to enable this feature. - `password` (String, Sensitive) Password for the default user. One of either `password` or `password_hash` must be specified. - `password_hash` (String, Sensitive) SHA256 hash of password for the default user. One of either `password` or `password_hash` must be specified. -- `private_endpoint_ids` (List of String) List of private endpoint IDs +- `private_endpoint_ids` (List of String, Deprecated) The `private_endpoint_ids` attribute is deprecated and not used. Please use `clickhouse_service_private_endpoint_attachment` resource instead. ### Read-Only diff --git a/pkg/internal/api/errors.go b/pkg/internal/api/errors.go new file mode 100644 index 0000000..02f2cbf --- /dev/null +++ b/pkg/internal/api/errors.go @@ -0,0 +1,13 @@ +package api + +import ( + "strings" +) + +func IsNotFound(err error) bool { + if err == nil { + return false + } + + return strings.HasPrefix(err.Error(), "status: 404") +} diff --git a/pkg/resource/models/service_resource.go b/pkg/resource/models/service_resource.go index 670dff0..9e1a405 100644 --- a/pkg/resource/models/service_resource.go +++ b/pkg/resource/models/service_resource.go @@ -112,7 +112,6 @@ func (m *ServiceResourceModel) Equals(b ServiceResourceModel) bool { !m.IdleTimeoutMinutes.Equal(b.IdleTimeoutMinutes) || !m.IAMRole.Equal(b.IAMRole) || !m.PrivateEndpointConfig.Equal(b.PrivateEndpointConfig) || - !m.PrivateEndpointIds.Equal(b.PrivateEndpointIds) || !m.EncryptionKey.Equal(b.EncryptionKey) || !m.EncryptionAssumedRoleIdentifier.Equal(b.EncryptionAssumedRoleIdentifier) || !m.IpAccessList.Equal(b.IpAccessList) { diff --git a/pkg/resource/models/service_resource_test.go b/pkg/resource/models/service_resource_test.go index d802b62..054dc5b 100644 --- a/pkg/resource/models/service_resource_test.go +++ b/pkg/resource/models/service_resource_test.go @@ -163,37 +163,6 @@ func TestServiceResource_Equals(t *testing.T) { }).Get(), want: false, }, - { - name: "PrivateEndpointIds added", - a: base, - b: test.NewUpdater(base).Update(func(src *ServiceResourceModel) { - existing := src.PrivateEndpointIds.Elements() - existing = append(existing, types.StringValue("added")) - privateEndpointIds, _ := types.ListValue(types.StringType, existing) - src.PrivateEndpointIds = privateEndpointIds - }).Get(), - want: false, - }, - { - name: "PrivateEndpointIds removed", - a: base, - b: test.NewUpdater(base).Update(func(src *ServiceResourceModel) { - existing := src.PrivateEndpointIds.Elements() - privateEndpointIds, _ := types.ListValue(types.StringType, []attr.Value{existing[0]}) - src.PrivateEndpointIds = privateEndpointIds - }).Get(), - want: false, - }, - { - name: "PrivateEndpointIds order changed", - a: base, - b: test.NewUpdater(base).Update(func(src *ServiceResourceModel) { - existing := src.PrivateEndpointIds.Elements() - privateEndpointIds, _ := types.ListValue(types.StringType, []attr.Value{existing[1], existing[0]}) - src.PrivateEndpointIds = privateEndpointIds - }).Get(), - want: false, - }, { name: "EncryptionKey changed", a: base, @@ -228,8 +197,6 @@ func getBaseModel() ServiceResourceModel { endpoints = append(endpoints, Endpoint{Protocol: types.StringValue("changed"), Host: types.StringValue("changed"), Port: types.Int64Value(1235)}.ObjectValue()) ep, _ := types.ListValue(Endpoint{}.ObjectType(), endpoints) - privateEndpointIds, _ := types.ListValueFrom(context.Background(), types.StringType, []string{"id1", "id2"}) - state := ServiceResourceModel{ ID: types.StringValue(uuid), Name: types.StringValue(""), @@ -248,7 +215,6 @@ func getBaseModel() ServiceResourceModel { IdleTimeoutMinutes: types.Int64{}, IAMRole: types.StringValue(""), PrivateEndpointConfig: PrivateEndpointConfig{}.ObjectValue(), - PrivateEndpointIds: privateEndpointIds, EncryptionKey: types.String{}, EncryptionAssumedRoleIdentifier: types.String{}, } diff --git a/pkg/resource/private_endpoint_registration.go b/pkg/resource/private_endpoint_registration.go index cd90601..6ce0900 100644 --- a/pkg/resource/private_endpoint_registration.go +++ b/pkg/resource/private_endpoint_registration.go @@ -126,19 +126,15 @@ func (r *PrivateEndpointRegistrationResource) Read(ctx context.Context, req reso } } - if privateEndpoint == nil { - resp.Diagnostics.AddError("Private endpoint not found", "Could not find private endpoint in org registration") - return - } - - state.Description = types.StringValue(privateEndpoint.Description) - state.Region = types.StringValue(privateEndpoint.Region) - state.CloudProvider = types.StringValue(privateEndpoint.CloudProvider) - - diags = resp.State.Set(ctx, &state) - resp.Diagnostics.Append(diags...) - if resp.Diagnostics.HasError() { - return + if privateEndpoint != nil { + state.Description = types.StringValue(privateEndpoint.Description) + state.Region = types.StringValue(privateEndpoint.Region) + state.CloudProvider = types.StringValue(privateEndpoint.CloudProvider) + + diags = resp.State.Set(ctx, &state) + resp.Diagnostics.Append(diags...) + } else { + resp.State.RemoveResource(ctx) } } diff --git a/pkg/resource/service.go b/pkg/resource/service.go index 4841dec..856fd9a 100644 --- a/pkg/resource/service.go +++ b/pkg/resource/service.go @@ -12,14 +12,12 @@ import ( "time" "github.com/ClickHouse/terraform-provider-clickhouse/pkg/internal/api" - "github.com/ClickHouse/terraform-provider-clickhouse/pkg/internal/tfutils" "github.com/ClickHouse/terraform-provider-clickhouse/pkg/resource/models" "github.com/hashicorp/terraform-plugin-framework/attr" "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/listdefault" "github.com/hashicorp/terraform-plugin-framework/resource/schema/listplanmodifier" "github.com/hashicorp/terraform-plugin-framework/resource/schema/objectplanmodifier" "github.com/hashicorp/terraform-plugin-framework/resource/schema/planmodifier" @@ -176,11 +174,10 @@ func (r *ServiceResource) Schema(_ context.Context, _ resource.SchemaRequest, re }, }, "private_endpoint_ids": schema.ListAttribute{ - Description: "List of private endpoint IDs", - ElementType: types.StringType, - Optional: true, - Computed: true, - Default: listdefault.StaticValue(tfutils.CreateEmptyList(types.StringType)), + Description: "The `private_endpoint_ids` attribute is deprecated and not used. Please use `clickhouse_service_private_endpoint_attachment` resource instead.", + ElementType: types.StringType, + Optional: true, + DeprecationMessage: "The `private_endpoint_ids` attribute is deprecated and not used. Please use `clickhouse_service_private_endpoint_attachment` resource instead.", }, "encryption_key": schema.StringAttribute{ Description: "Custom encryption key arn", @@ -412,12 +409,6 @@ func (r *ServiceResource) Create(ctx context.Context, req resource.CreateRequest } service.IpAccessList = ipAccessLists - servicePrivateEndpointIds := make([]types.String, 0, len(plan.PrivateEndpointIds.Elements())) - plan.PrivateEndpointIds.ElementsAs(ctx, &servicePrivateEndpointIds, false) - for _, item := range servicePrivateEndpointIds { - service.PrivateEndpointIds = append(service.PrivateEndpointIds, item.ValueString()) - } - // Create new service s, _, err := r.client.CreateService(service) if err != nil { @@ -522,11 +513,13 @@ func (r *ServiceResource) Read(ctx context.Context, req resource.ReadRequest, re return } - // Set refreshed state - diags = resp.State.Set(ctx, &state) - resp.Diagnostics.Append(diags...) - if resp.Diagnostics.HasError() { - return + if state.ID.IsNull() { + // Resource was deleted outside terraform + resp.State.RemoveResource(ctx) + } else { + // Set refreshed state + diags = resp.State.Set(ctx, state) + resp.Diagnostics.Append(diags...) } } @@ -584,30 +577,6 @@ func (r *ServiceResource) Update(ctx context.Context, req resource.UpdateRequest } } - if !plan.PrivateEndpointIds.Equal(state.PrivateEndpointIds) { - serviceChange = true - privateEndpointIdsRawOld := make([]types.String, 0, len(state.PrivateEndpointIds.Elements())) - privateEndpointIdsRawNew := make([]types.String, 0, len(plan.PrivateEndpointIds.Elements())) - state.PrivateEndpointIds.ElementsAs(ctx, &privateEndpointIdsRawOld, false) - plan.PrivateEndpointIds.ElementsAs(ctx, &privateEndpointIdsRawNew, false) - - privateEndpointIdsOld := []string{} - privateEndpointIdsNew := []string{} - - for _, item := range privateEndpointIdsRawOld { - privateEndpointIdsOld = append(privateEndpointIdsOld, item.ValueString()) - } - - for _, item := range privateEndpointIdsRawNew { - privateEndpointIdsNew = append(privateEndpointIdsNew, item.ValueString()) - } - - service.PrivateEndpointIds = &api.PrivateEndpointIdsUpdate{ - Add: privateEndpointIdsNew, - Remove: privateEndpointIdsOld, - } - } - // Update existing service if serviceChange { var err error @@ -757,7 +726,12 @@ func (r *ServiceResource) syncServiceState(ctx context.Context, state *models.Se // Get latest service value from ClickHouse OpenAPI service, err := r.client.GetService(state.ID.ValueString()) - if err != nil { + if api.IsNotFound(err) { + // Service was deleted outside terraform. + state.ID = types.StringNull() + + return nil + } else if err != nil { return err } @@ -821,12 +795,6 @@ func (r *ServiceResource) syncServiceState(ctx context.Context, state *models.Se state.EncryptionAssumedRoleIdentifier = types.StringNull() } - if len(service.PrivateEndpointIds) == 0 { - state.PrivateEndpointIds = tfutils.CreateEmptyList(types.StringType) - } else { - state.PrivateEndpointIds, _ = types.ListValueFrom(ctx, types.StringType, service.PrivateEndpointIds) - } - return nil } diff --git a/pkg/resource/service_test.go b/pkg/resource/service_test.go index 958d09c..ae32731 100644 --- a/pkg/resource/service_test.go +++ b/pkg/resource/service_test.go @@ -308,21 +308,6 @@ func TestServiceResource_syncServiceState(t *testing.T) { updateTimestamp: false, wantErr: false, }, - { - name: "Update PrivateEndpointIds field", - state: state, - response: test.NewUpdater(getBaseResponse(state.ID.ValueString())).Update(func(src *api.Service) { - src.PrivateEndpointIds = []string{ - "newendpointid", - } - }).GetPtr(), - responseErr: nil, - desiredState: test.NewUpdater(state).Update(func(src *models.ServiceResourceModel) { - src.PrivateEndpointIds, _ = types.ListValueFrom(context.Background(), types.StringType, []string{"newendpointid"}) - }).Get(), - updateTimestamp: false, - wantErr: false, - }, { name: "Update EncryptionKey field", state: state, @@ -412,7 +397,6 @@ func getInitialState() models.ServiceResourceModel { EndpointServiceID: types.StringValue(""), PrivateDNSHostname: types.StringValue(""), }.ObjectValue() - privateEndpointIds, _ := types.ListValue(types.StringType, []attr.Value{}) state := models.ServiceResourceModel{ ID: types.StringValue(uuid), @@ -432,7 +416,6 @@ func getInitialState() models.ServiceResourceModel { IdleTimeoutMinutes: types.Int64{}, IAMRole: types.StringValue(""), PrivateEndpointConfig: privateEndpointConfig, - PrivateEndpointIds: privateEndpointIds, EncryptionKey: types.StringNull(), EncryptionAssumedRoleIdentifier: types.StringNull(), } @@ -460,7 +443,6 @@ func getBaseResponse(id string) api.Service { EndpointServiceId: "", PrivateDnsHostname: "", }, - // PrivateEndpointIds: nil, // EncryptionKey: "", // EncryptionAssumedRoleIdentifier: "", }