Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Dont overwrite deployment id with self #789

Merged
merged 5 commits into from
Mar 12, 2024
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -36,13 +36,15 @@ import (
enterprisesearchv2 "github.com/elastic/terraform-provider-ec/ec/ecresource/deploymentresource/enterprisesearch/v2"
integrationsserverv2 "github.com/elastic/terraform-provider-ec/ec/ecresource/deploymentresource/integrationsserver/v2"
kibanav2 "github.com/elastic/terraform-provider-ec/ec/ecresource/deploymentresource/kibana/v2"
v1 "github.com/elastic/terraform-provider-ec/ec/ecresource/deploymentresource/observability/v1"
observabilityv2 "github.com/elastic/terraform-provider-ec/ec/ecresource/deploymentresource/observability/v2"
"github.com/elastic/terraform-provider-ec/ec/ecresource/deploymentresource/utils"
"github.com/elastic/terraform-provider-ec/ec/internal/converters"
"github.com/elastic/terraform-provider-ec/ec/internal/util"
"github.com/hashicorp/terraform-plugin-framework/diag"
"github.com/hashicorp/terraform-plugin-framework/tfsdk"
"github.com/hashicorp/terraform-plugin-framework/types"
"github.com/hashicorp/terraform-plugin-framework/types/basetypes"
)

type Deployment struct {
Expand Down Expand Up @@ -295,19 +297,34 @@ func (dep *Deployment) parseCredentials(resources []*models.DeploymentResource)
}
}

func (dep *Deployment) ProcessSelfInObservability() {

if dep.Observability == nil {
return
func (dep *Deployment) ProcessSelfInObservability(ctx context.Context, base DeploymentTF) diag.Diagnostics {
if dep == nil || dep.Observability == nil {
return nil
}

if dep.Observability.DeploymentId == nil {
return
return nil
}

var baseObservability v1.ObservabilityTF
diags := base.Observability.As(ctx, &baseObservability, basetypes.ObjectAsOptions{
UnhandledNullAsEmpty: true,
UnhandledUnknownAsEmpty: true,
dimuon marked this conversation as resolved.
Show resolved Hide resolved
})
if diags.HasError() {
return diags
}

deploymentIDIsKnown := !(baseObservability.DeploymentId.IsNull() || baseObservability.DeploymentId.IsUnknown())
if deploymentIDIsKnown && baseObservability.DeploymentId.ValueString() != "self" {
return nil
}

if *dep.Observability.DeploymentId == dep.Id {
*dep.Observability.DeploymentId = "self"
}

return nil
}

func (dep *Deployment) IncludePrivateStateTrafficFilters(ctx context.Context, base DeploymentTF, privateFilters []string) diag.Diagnostics {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ import (
enterprisesearchv2 "github.com/elastic/terraform-provider-ec/ec/ecresource/deploymentresource/enterprisesearch/v2"
kibanav2 "github.com/elastic/terraform-provider-ec/ec/ecresource/deploymentresource/kibana/v2"
observabilityv2 "github.com/elastic/terraform-provider-ec/ec/ecresource/deploymentresource/observability/v2"
"github.com/hashicorp/terraform-plugin-framework/attr"
"github.com/hashicorp/terraform-plugin-framework/tfsdk"
"github.com/hashicorp/terraform-plugin-framework/types"
"github.com/stretchr/testify/assert"
Expand Down Expand Up @@ -1729,6 +1730,132 @@ func Test_getDeploymentTemplateID(t *testing.T) {
}
}

func Test_ProcessSelfInObservability(t *testing.T) {
tests := []struct {
name string
deployment *Deployment
baseObservability *observabilityv2.Observability
observabilityIsUnknown bool
expectedObservabilityDeploymentID *string
expectNonNilDiags bool
}{
{
name: "should noop if deployment is nil",
},
{name: "should noop if observability section is nil",
deployment: &Deployment{},
},
{
name: "should noop if observability deployment id is nil",
deployment: &Deployment{
Observability: &observabilityv2.Observability{},
},
},
{
name: "should not change the observability deployment id to self if it equals the deployment id and the configured value is not self",
deployment: &Deployment{
Id: "deployment-id",
Observability: &observabilityv2.Observability{
DeploymentId: ec.String("deployment-id"),
},
},
baseObservability: &observabilityv2.Observability{
DeploymentId: ec.String("deployment-id"),
},
expectedObservabilityDeploymentID: ec.String("deployment-id"),
},
{
name: "should set observability deployment id to self if it equals the deployment id and the configured value is self",
deployment: &Deployment{
Id: "deployment-id",
Observability: &observabilityv2.Observability{
DeploymentId: ec.String("deployment-id"),
},
},
baseObservability: &observabilityv2.Observability{
DeploymentId: ec.String("self"),
},
expectedObservabilityDeploymentID: ec.String("self"),
},
{
name: "should set observability deployment id to self if it equals the deployment id and the configured value is not set",
deployment: &Deployment{
Id: "deployment-id",
Observability: &observabilityv2.Observability{
DeploymentId: ec.String("deployment-id"),
},
},
baseObservability: &observabilityv2.Observability{},
expectedObservabilityDeploymentID: ec.String("self"),
},
{
name: "should not change the observability deployment id if it does not equal the deployment id",
deployment: &Deployment{
Id: "deployment-id",
Observability: &observabilityv2.Observability{
DeploymentId: ec.String("another-deployment-id"),
},
},
expectedObservabilityDeploymentID: ec.String("another-deployment-id"),
},
{
name: "should set observability deployment id to self if it equals the deployment id and no observability is configured",
deployment: &Deployment{
Id: "deployment-id",
Observability: &observabilityv2.Observability{
DeploymentId: ec.String("deployment-id"),
},
},
expectedObservabilityDeploymentID: ec.String("self"),
},
{
name: "should set observability deployment id to self if it equals the deployment id and the configured value is unknown",
deployment: &Deployment{
Id: "deployment-id",
Observability: &observabilityv2.Observability{
DeploymentId: ec.String("deployment-id"),
},
},
observabilityIsUnknown: true,
expectedObservabilityDeploymentID: ec.String("self"),
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
var obj types.Object
observabilitySchema := observabilityv2.ObservabilitySchema().GetType()
schemaWithAttrs, ok := observabilitySchema.(attr.TypeWithAttributeTypes)
require.True(t, ok)

if tt.baseObservability != nil {
tobio marked this conversation as resolved.
Show resolved Hide resolved
diags := tfsdk.ValueFrom(context.Background(), tt.baseObservability, observabilitySchema, &obj)
require.Nil(t, diags)
} else if tt.observabilityIsUnknown {
obj = types.ObjectUnknown(schemaWithAttrs.AttributeTypes())
} else {
obj = types.ObjectNull(schemaWithAttrs.AttributeTypes())
}

baseDeployment := DeploymentTF{Observability: obj}

diags := tt.deployment.ProcessSelfInObservability(context.Background(), baseDeployment)
if tt.expectNonNilDiags {
require.NotNil(t, diags)
} else {
require.Nil(t, diags)
}

var finalObservabilityDeploymentID *string
if tt.deployment != nil && tt.deployment.Observability != nil {
finalObservabilityDeploymentID = tt.deployment.Observability.DeploymentId
}

require.Equal(t, tt.expectedObservabilityDeploymentID, finalObservabilityDeploymentID)
})
}
}

func Test_PersistSnapshotSource(t *testing.T) {
tests := []struct {
name string
Expand Down
2 changes: 1 addition & 1 deletion ec/ecresource/deploymentresource/read.go
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@ func (r *Resource) read(ctx context.Context, id string, state *deploymentv2.Depl

deployment.SetCredentialsIfEmpty(state)

deployment.ProcessSelfInObservability()
diags.Append(deployment.ProcessSelfInObservability(ctx, base)...)

deployment.NullifyUnusedEsTopologies(ctx, baseElasticsearch)
diags.Append(deployment.PersistSnapshotSource(ctx, baseElasticsearch)...)
Expand Down
Loading