correctly handle alias slugs on aliases reconciliation (#387)

* correctly handle alias slugs on aliases reconciliation * updating aliases can delete all non-default aliases * update comments * bump opslevel-go version to v2024.7.2
OpsLevel · Jul 2, 2024 · 2bb83d1 · 2bb83d1
1 parent 7a2141d
commit 2bb83d1
Show file tree

Hide file tree

Showing 9 changed files with 125 additions and 90 deletions.
diff --git a/.changes/unreleased/Dependency-20240702-120053.yaml b/.changes/unreleased/Dependency-20240702-120053.yaml
@@ -0,0 +1,3 @@
+kind: Dependency
+body: bump opslevel-go version to v2024.7.2
+time: 2024-07-02T12:00:53.980968-05:00
diff --git a/go.mod b/go.mod
@@ -6,7 +6,7 @@ require (
 	github.com/hashicorp/terraform-plugin-framework v1.9.0
 	github.com/hashicorp/terraform-plugin-framework-validators v0.12.0
 	github.com/hashicorp/terraform-plugin-log v0.9.0
-	github.com/opslevel/opslevel-go/v2024 v2024.6.27
+	github.com/opslevel/opslevel-go/v2024 v2024.7.2
 	github.com/relvacode/iso8601 v1.4.0
 	golang.org/x/net v0.26.0
 )

diff --git a/go.sum b/go.sum
@@ -92,8 +92,8 @@ github.com/oklog/run v1.1.0 h1:GEenZ1cK0+q0+wsJew9qUg/DyD8k3JzYsZAi5gYi2mA=
 github.com/oklog/run v1.1.0/go.mod h1:sVPdnTZT1zYwAJeCMu2Th4T21pA3FPOQRfWjQlk7DVU=
 github.com/opslevel/moredefaults v0.0.0-20240529152742-17d1318a3c12 h1:OQZ3W8kbyCcdS8QUWFTnZd6xtdkfhdckc7Paro7nXio=
 github.com/opslevel/moredefaults v0.0.0-20240529152742-17d1318a3c12/go.mod h1:g2GSXVP6LO+5+AIsnMRPN+BeV86OXuFRTX7HXCDtYeI=
-github.com/opslevel/opslevel-go/v2024 v2024.6.27 h1:93eJMfPn0rFKtW4cKmB3tskVSQ3rrqbyWrktcFBj24Q=
-github.com/opslevel/opslevel-go/v2024 v2024.6.27/go.mod h1:FsJFzudwLP7FIPxtlJw7MLaQHyScyfzkzBoj6k07wc0=
+github.com/opslevel/opslevel-go/v2024 v2024.7.2 h1:bZhvTVWbpJSvxcc9ymsBaxNaC3i+sIrf0ZQ3gLaVifU=
+github.com/opslevel/opslevel-go/v2024 v2024.7.2/go.mod h1:FsJFzudwLP7FIPxtlJw7MLaQHyScyfzkzBoj6k07wc0=
 github.com/pkg/errors v0.9.1 h1:FEBLx1zS214owpjy7qsBeixbURkuhQAwrK5UwLGTwt4=
 github.com/pkg/errors v0.9.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0=
 github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM=

diff --git a/opslevel/common.go b/opslevel/common.go
@@ -11,7 +11,6 @@ import (
 	"github.com/hashicorp/terraform-plugin-framework-validators/stringvalidator"
 	"github.com/hashicorp/terraform-plugin-framework/datasource"
 	"github.com/hashicorp/terraform-plugin-framework/datasource/schema"
-	"github.com/hashicorp/terraform-plugin-framework/diag"
 	"github.com/hashicorp/terraform-plugin-framework/resource"
 	"github.com/hashicorp/terraform-plugin-framework/schema/validator"
 	"github.com/hashicorp/terraform-plugin-framework/types"
@@ -133,35 +132,6 @@ func diffBetweenStringSlices(sliceOne, sliceTwo []string) []string {
 	return diffValues
 }
 
-// converts resourceAliases to SetValue for resourceModels. validates modelAliases contains slugs if not empty
-func stringAliasesToSetValue(ctx context.Context, resourceAliases []string, modelAliases basetypes.SetValue) (basetypes.SetValue, diag.Diagnostics) {
-	aliases := types.SetNull(types.StringType)
-
-	// config has `aliases = null` or omitted
-	if modelAliases.IsNull() {
-		return aliases, nil
-	}
-	// config has `aliases = []` - not null
-	if len(modelAliases.Elements()) == 0 {
-		return types.SetValueFrom(ctx, types.StringType, modelAliases)
-	}
-
-	// check if config has 'aliases' set, but is missing needed alias "slugs" from API
-	aliasesFromModel, diags := SetValueToStringSlice(ctx, modelAliases)
-	if diags != nil && diags.HasError() {
-		return aliases, diags
-	}
-	aliasesNeededInConfig := diffBetweenStringSlices(resourceAliases, aliasesFromModel)
-	if len(aliasesNeededInConfig) > 0 {
-		// setting aliases here blocks an erroneous error. The "Config error" is what matters
-		diags.AddError("Config error", fmt.Sprintf(`default aliases from API need to be added to config: %s`, aliasesNeededInConfig))
-		return aliases, diags
-	}
-
-	// aliases is set and contains alias "slugs" from API
-	return types.SetValueFrom(ctx, types.StringType, resourceAliases)
-}
-
 // getDatasourceFilter originally had a "required" bool input parameter - no longer needed
 func getDatasourceFilter(validFieldNames []string) schema.SingleNestedBlock {
 	return schema.SingleNestedBlock{

diff --git a/opslevel/resource_opslevel_infra.go b/opslevel/resource_opslevel_infra.go
@@ -4,7 +4,6 @@ import (
 	"context"
 	"fmt"
 
-	"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"
@@ -57,8 +56,7 @@ type InfrastructureResourceModel struct {
 	Schema       types.String       `tfsdk:"schema"`
 }
 
-func NewInfrastructureResourceModel(ctx context.Context, infrastructure opslevel.InfrastructureResource, givenModel InfrastructureResourceModel) (InfrastructureResourceModel, diag.Diagnostics) {
-	var diags diag.Diagnostics
+func NewInfrastructureResourceModel(ctx context.Context, infrastructure opslevel.InfrastructureResource, givenModel InfrastructureResourceModel) InfrastructureResourceModel {
 	var providerData *InfraProviderData
 
 	if infrastructure.ProviderData.AccountName != "" {
@@ -72,9 +70,13 @@ func NewInfrastructureResourceModel(ctx context.Context, infrastructure opslevel
 		Owner:        RequiredStringValue(string(infrastructure.Owner.Id())),
 		Schema:       RequiredStringValue(infrastructure.Schema),
 	}
-	infrastructureResourceModel.Aliases, diags = stringAliasesToSetValue(ctx, infrastructure.Aliases, givenModel.Aliases)
+	if givenModel.Aliases.IsNull() {
+		infrastructureResourceModel.Aliases = types.SetNull(types.StringType)
+	} else {
+		infrastructureResourceModel.Aliases = givenModel.Aliases
+	}
 
-	return infrastructureResourceModel, diags
+	return infrastructureResourceModel
 }
 
 func (r *InfrastructureResource) Metadata(ctx context.Context, req resource.MetadataRequest, resp *resource.MetadataResponse) {
@@ -166,16 +168,21 @@ func (r *InfrastructureResource) Create(ctx context.Context, req resource.Create
 	if len(planModel.Aliases.Elements()) > 0 {
 		aliases, diags := SetValueToStringSlice(ctx, planModel.Aliases)
 		if diags != nil && diags.HasError() {
-			resp.Diagnostics.AddError("Config error", fmt.Sprintf("Unable to handle given infrastructure aliases: '%s'", planModel.Aliases))
+			resp.Diagnostics.Append(diags...)
+			resp.Diagnostics.AddAttributeError(path.Root("aliases"), "Config error", "unable to handle given infrastructure aliases")
 			return
 		}
 		if err = infrastructure.ReconcileAliases(r.client, aliases); err != nil {
-			resp.Diagnostics.AddWarning("opslevel client error", fmt.Sprintf("Unable to reconcile infrastructure aliases: '%s'\n%s", aliases, err))
+			resp.Diagnostics.AddError("opslevel client error", fmt.Sprintf("Unable to reconcile infrastructure aliases: '%s'\n%s", aliases, err))
+
+			// delete newly created infrastructure to avoid dupliate infrastructure creation on next 'terraform apply'
+			if err := r.client.DeleteInfrastructure(infrastructure.Id); err != nil {
+				resp.Diagnostics.AddError("opslevel client error", fmt.Sprintf("failed to delete incorrectly created infrastructure '%s' following aliases error:\n%s", infrastructure.Name, err))
+			}
 		}
 	}
 
-	createdInfrastructureResourceModel, diags := NewInfrastructureResourceModel(ctx, *infrastructure, planModel)
-	resp.Diagnostics.Append(diags...)
+	createdInfrastructureResourceModel := NewInfrastructureResourceModel(ctx, *infrastructure, planModel)
 
 	tflog.Trace(ctx, "created a infrastructure resource")
 	resp.Diagnostics.Append(resp.State.Set(ctx, &createdInfrastructureResourceModel)...)
@@ -196,8 +203,7 @@ func (r *InfrastructureResource) Read(ctx context.Context, req resource.ReadRequ
 		resp.Diagnostics.AddError("opslevel client error", fmt.Sprintf("Unable to read infrastructure, got error: %s", err))
 		return
 	}
-	readInfrastructureResourceModel, diags := NewInfrastructureResourceModel(ctx, *infrastructure, stateModel)
-	resp.Diagnostics.Append(diags...)
+	readInfrastructureResourceModel := NewInfrastructureResourceModel(ctx, *infrastructure, stateModel)
 
 	// Save updated data into Terraform state
 	resp.Diagnostics.Append(resp.State.Set(ctx, &readInfrastructureResourceModel)...)
@@ -225,15 +231,16 @@ func (r *InfrastructureResource) Update(ctx context.Context, req resource.Update
 
 	givenAliases, diags := SetValueToStringSlice(ctx, planModel.Aliases)
 	if diags != nil && diags.HasError() {
-		resp.Diagnostics.AddError("Config error", fmt.Sprintf("Unable to handle given infrastructure aliases: '%s'", planModel.Aliases))
+		resp.Diagnostics.Append(diags...)
+		resp.Diagnostics.AddAttributeError(path.Root("aliases"), "Config error", "unable to handle given infrastructure aliases")
 		return
 	}
 	if err = updatedInfrastructure.ReconcileAliases(r.client, givenAliases); err != nil {
-		resp.Diagnostics.AddWarning("opslevel client error", fmt.Sprintf("Unable to reconcile infrastructure aliases: '%s'\n%s", givenAliases, err))
+		resp.Diagnostics.AddError("opslevel client error", fmt.Sprintf("Unable to reconcile infrastructure aliases: '%s'\n%s", givenAliases, err))
+		return
 	}
 
-	updatedInfrastructureResourceModel, diags := NewInfrastructureResourceModel(ctx, *updatedInfrastructure, planModel)
-	resp.Diagnostics.Append(diags...)
+	updatedInfrastructureResourceModel := NewInfrastructureResourceModel(ctx, *updatedInfrastructure, planModel)
 
 	if planModel.ProviderData == nil && updatedInfrastructureResourceModel.ProviderData != nil {
 		resp.Diagnostics.AddError("Known error", "Unable to unset 'provider_data' field for now. We have a planned fix for this.")

diff --git a/opslevel/resource_opslevel_service.go b/opslevel/resource_opslevel_service.go
@@ -4,6 +4,7 @@ import (
 	"context"
 	"fmt"
 	"regexp"
+	"slices"
 	"strings"
 
 	"github.com/hashicorp/terraform-plugin-framework-validators/stringvalidator"
@@ -64,9 +65,10 @@ func newServiceResourceModel(ctx context.Context, service opslevel.Service, give
 		TierAlias:       OptionalStringValue(service.Tier.Alias),
 	}
 
-	serviceResourceModel.Aliases, diags = stringAliasesToSetValue(ctx, service.Aliases, givenModel.Aliases)
-	if diags != nil && diags.HasError() {
-		return serviceResourceModel, diags
+	if givenModel.Aliases.IsNull() {
+		serviceResourceModel.Aliases = types.SetNull(types.StringType)
+	} else {
+		serviceResourceModel.Aliases = givenModel.Aliases
 	}
 
 	if givenModel.Tags.IsNull() && (service.Tags != nil || len(service.Tags.Nodes) == 0) {
@@ -216,20 +218,30 @@ func (r *ServiceResource) Create(ctx context.Context, req resource.CreateRequest
 	}
 
 	// TODO: the post create/update steps are the same and can be extracted into a function so we repeat less code
-	givenAliases, diags := SetValueToStringSlice(ctx, planModel.Aliases)
-	if diags != nil && diags.HasError() {
-		resp.Diagnostics.AddError("Config error", fmt.Sprintf("Unable to handle given service aliases: '%s'", planModel.Aliases))
-		return
-	}
-	if len(givenAliases) > 0 {
-		if err = service.ReconcileAliases(r.client, givenAliases); err != nil {
-			resp.Diagnostics.AddWarning("opslevel client error", fmt.Sprintf("Unable to reconcile service aliases: '%s'\n%s", givenAliases, err))
-			resp.Diagnostics.AddWarning("Config warning", "On create, OpsLevel API creates a new alias for services. If this causes issues, create team with empty 'aliases'. Then update team with 'aliases'")
+	if len(planModel.Aliases.Elements()) > 0 {
+		aliases, diags := SetValueToStringSlice(ctx, planModel.Aliases)
+		if diags != nil && diags.HasError() {
+			resp.Diagnostics.Append(diags...)
+			resp.Diagnostics.AddAttributeError(path.Root("aliases"), "Config error", "unable to handle given service aliases")
+			return
+		}
+		// add "unique identifiers" (OpsLevel created aliases) before reconciling.
+		// this ensures that we don't try to create an alias that already exists
+		aliases = append(aliases, service.UniqueIdentifiers()...)
+		if err = service.ReconcileAliases(r.client, aliases); err != nil {
+			resp.Diagnostics.AddError("opslevel client error", fmt.Sprintf("Unable to reconcile service aliases: '%s'\n%s", aliases, err))
+
+			// delete newly created team to avoid dupliate team creation on next 'terraform apply'
+			if err := r.client.DeleteService(string(service.Id)); err != nil {
+				resp.Diagnostics.AddError("opslevel client error", fmt.Sprintf("failed to delete incorrectly created service '%s' following aliases error:\n%s", service.Name, err))
+			}
+			return
 		}
 	}
 
 	givenTags, diags := TagSetValueToTagSlice(ctx, planModel.Tags)
 	if diags != nil && diags.HasError() {
+		resp.Diagnostics.Append(diags...)
 		resp.Diagnostics.AddError("Config error", fmt.Sprintf("Unable to handle given service tags: '%s'", planModel.Tags))
 		return
 	}
@@ -328,20 +340,35 @@ func (r *ServiceResource) Update(ctx context.Context, req resource.UpdateRequest
 		return
 	}
 
-	if len(planModel.Aliases.Elements()) > 0 {
-		aliases, diags := SetValueToStringSlice(ctx, planModel.Aliases)
-		if diags != nil && diags.HasError() {
-			resp.Diagnostics.AddError("Config error", fmt.Sprintf("Unable to handle given service aliases: '%s'", planModel.Aliases))
-			return
-		}
-		if err = service.ReconcileAliases(r.client, aliases); err != nil {
-			resp.Diagnostics.AddWarning("opslevel client error", fmt.Sprintf("Unable to reconcile service aliases: '%s'\n%s", aliases, err))
+	aliases, diags := SetValueToStringSlice(ctx, planModel.Aliases)
+	if diags != nil && diags.HasError() {
+		resp.Diagnostics.Append(diags...)
+		resp.Diagnostics.AddAttributeError(path.Root("aliases"), "Config error", "unable to handle given service aliases")
+		return
+	}
+
+	// Try deleting uniqueIdentifiers (aka default alias) if not declared in Terraform config
+	// Deleting this alias may fail because its locked but that's ok
+	uniqueIdentifiers := service.UniqueIdentifiers()
+	for _, uniqueIdentifier := range uniqueIdentifiers {
+		if !slices.Contains(aliases, uniqueIdentifier) {
+			_ = r.client.DeleteAlias(opslevel.AliasDeleteInput{
+				Alias:     uniqueIdentifier,
+				OwnerType: opslevel.AliasOwnerTypeEnumService,
+			})
 		}
 	}
+	// add "unique identifiers" (OpsLevel created aliases) before reconciling.
+	// this ensures that we don't try to create an alias that already exists
+	aliases = append(aliases, uniqueIdentifiers...)
+	if err = service.ReconcileAliases(r.client, aliases); err != nil {
+		resp.Diagnostics.AddError("opslevel client error", fmt.Sprintf("Unable to reconcile service aliases: '%s'\n%s", aliases, err))
+	}
 
 	givenTags, diags := TagSetValueToTagSlice(ctx, planModel.Tags)
 	if diags != nil && diags.HasError() {
-		resp.Diagnostics.AddError("Config error", fmt.Sprintf("Unable to handle given service tags: '%s'", planModel.Tags))
+		resp.Diagnostics.Append(diags...)
+		resp.Diagnostics.AddAttributeError(path.Root("tags"), "Config error", "unable to handle given service tags")
 		return
 	}
 	if err = r.client.ReconcileTags(service, givenTags); err != nil {