Skip to content

Commit

Permalink
Remove private endpoints from service (#138)
Browse files Browse the repository at this point in the history
  • Loading branch information
whites11 authored Aug 20, 2024
1 parent 8fad605 commit ed80cb1
Show file tree
Hide file tree
Showing 9 changed files with 43 additions and 117 deletions.
2 changes: 2 additions & 0 deletions .github/workflows/e2e.yaml
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
name: E2E tests

on:
workflow_dispatch:
inputs: {}
schedule:
- cron: "0 3 * * *"

Expand Down
2 changes: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion docs/resources/service.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
13 changes: 13 additions & 0 deletions pkg/internal/api/errors.go
Original file line number Diff line number Diff line change
@@ -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")
}
1 change: 0 additions & 1 deletion pkg/resource/models/service_resource.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
34 changes: 0 additions & 34 deletions pkg/resource/models/service_resource_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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(""),
Expand All @@ -248,7 +215,6 @@ func getBaseModel() ServiceResourceModel {
IdleTimeoutMinutes: types.Int64{},
IAMRole: types.StringValue(""),
PrivateEndpointConfig: PrivateEndpointConfig{}.ObjectValue(),
PrivateEndpointIds: privateEndpointIds,
EncryptionKey: types.String{},
EncryptionAssumedRoleIdentifier: types.String{},
}
Expand Down
22 changes: 9 additions & 13 deletions pkg/resource/private_endpoint_registration.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}

Expand Down
66 changes: 17 additions & 49 deletions pkg/resource/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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...)
}
}

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
}

Expand Down Expand Up @@ -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
}

Expand Down
18 changes: 0 additions & 18 deletions pkg/resource/service_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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),
Expand All @@ -432,7 +416,6 @@ func getInitialState() models.ServiceResourceModel {
IdleTimeoutMinutes: types.Int64{},
IAMRole: types.StringValue(""),
PrivateEndpointConfig: privateEndpointConfig,
PrivateEndpointIds: privateEndpointIds,
EncryptionKey: types.StringNull(),
EncryptionAssumedRoleIdentifier: types.StringNull(),
}
Expand Down Expand Up @@ -460,7 +443,6 @@ func getBaseResponse(id string) api.Service {
EndpointServiceId: "",
PrivateDnsHostname: "",
},
// PrivateEndpointIds: nil,
// EncryptionKey: "",
// EncryptionAssumedRoleIdentifier: "",
}
Expand Down

0 comments on commit ed80cb1

Please sign in to comment.