From d37b60a1c6453f31d52897cbabd403b6f8a337c8 Mon Sep 17 00:00:00 2001 From: Toby Brain Date: Wed, 14 Sep 2022 16:29:04 +1000 Subject: [PATCH 1/8] Ignore fields explicitly set to null in Kibana resources --- .../deploymentresource/kibana_expanders.go | 68 +++++++++---------- .../kibana_expanders_test.go | 39 +++++++++++ ec/internal/util/parsers.go | 30 ++++---- 3 files changed, 86 insertions(+), 51 deletions(-) diff --git a/ec/ecresource/deploymentresource/kibana_expanders.go b/ec/ecresource/deploymentresource/kibana_expanders.go index f5a4caea0..0133badb7 100644 --- a/ec/ecresource/deploymentresource/kibana_expanders.go +++ b/ec/ecresource/deploymentresource/kibana_expanders.go @@ -53,18 +53,16 @@ func expandKibanaResources(kibanas []interface{}, tpl *models.KibanaPayload) ([] func expandKibanaResource(raw interface{}, res *models.KibanaPayload) (*models.KibanaPayload, error) { kibana := raw.(map[string]interface{}) - if esRefID, ok := kibana["elasticsearch_cluster_ref_id"]; ok { - res.ElasticsearchClusterRefID = ec.String(esRefID.(string)) + if esRefID, ok := kibana["elasticsearch_cluster_ref_id"].(string); ok { + res.ElasticsearchClusterRefID = ec.String(esRefID) } - if refID, ok := kibana["ref_id"]; ok { - res.RefID = ec.String(refID.(string)) + if refID, ok := kibana["ref_id"].(string); ok { + res.RefID = ec.String(refID) } - if region, ok := kibana["region"]; ok { - if r := region.(string); r != "" { - res.Region = ec.String(r) - } + if region, ok := kibana["region"].(string); ok && region != "" { + res.Region = ec.String(region) } if cfg, ok := kibana["config"]; ok { @@ -73,7 +71,7 @@ func expandKibanaResource(raw interface{}, res *models.KibanaPayload) (*models.K } } - if rt, ok := kibana["topology"]; ok && len(rt.([]interface{})) > 0 { + if rt, ok := kibana["topology"].([]interface{}); ok && len(rt) > 0 { topology, err := expandKibanaTopology(rt, res.Plan.ClusterTopology) if err != nil { return nil, err @@ -86,14 +84,17 @@ func expandKibanaResource(raw interface{}, res *models.KibanaPayload) (*models.K return res, nil } -func expandKibanaTopology(raw interface{}, topologies []*models.KibanaClusterTopologyElement) ([]*models.KibanaClusterTopologyElement, error) { - var rawTopologies = raw.([]interface{}) +func expandKibanaTopology(rawTopologies []interface{}, topologies []*models.KibanaClusterTopologyElement) ([]*models.KibanaClusterTopologyElement, error) { var res = make([]*models.KibanaClusterTopologyElement, 0, len(rawTopologies)) for i, rawTop := range rawTopologies { - var topology = rawTop.(map[string]interface{}) + var topology, ok = rawTop.(map[string]interface{}) + if !ok { + continue + } + var icID string - if id, ok := topology["instance_configuration_id"]; ok { - icID = id.(string) + if id, ok := topology["instance_configuration_id"].(string); ok { + icID = id } // When a topology element is set but no instance_configuration_id // is set, then obtain the instance_configuration_id from the topology @@ -114,10 +115,8 @@ func expandKibanaTopology(raw interface{}, topologies []*models.KibanaClusterTop elem.Size = size } - if zones, ok := topology["zone_count"]; ok { - if z := zones.(int); z > 0 { - elem.ZoneCount = int32(z) - } + if zones, ok := topology["zone_count"].(int); ok && zones > 0 { + elem.ZoneCount = int32(zones) } res = append(res, elem) @@ -128,30 +127,29 @@ func expandKibanaTopology(raw interface{}, topologies []*models.KibanaClusterTop func expandKibanaConfig(raw interface{}, res *models.KibanaConfiguration) error { for _, rawCfg := range raw.([]interface{}) { - var cfg = rawCfg.(map[string]interface{}) - if settings, ok := cfg["user_settings_json"]; ok && settings != nil { - if s, ok := settings.(string); ok && s != "" { - if err := json.Unmarshal([]byte(s), &res.UserSettingsJSON); err != nil { - return fmt.Errorf("failed expanding kibana user_settings_json: %w", err) - } + cfg, ok := rawCfg.(map[string]interface{}) + if !ok { + continue + } + if settings, ok := cfg["user_settings_json"].(string); ok && settings != "" { + if err := json.Unmarshal([]byte(settings), &res.UserSettingsJSON); err != nil { + return fmt.Errorf("failed expanding kibana user_settings_json: %w", err) } } - if settings, ok := cfg["user_settings_override_json"]; ok && settings != nil { - if s, ok := settings.(string); ok && s != "" { - if err := json.Unmarshal([]byte(s), &res.UserSettingsOverrideJSON); err != nil { - return fmt.Errorf("failed expanding kibana user_settings_override_json: %w", err) - } + if settings, ok := cfg["user_settings_override_json"].(string); ok && settings != "" { + if err := json.Unmarshal([]byte(settings), &res.UserSettingsOverrideJSON); err != nil { + return fmt.Errorf("failed expanding kibana user_settings_override_json: %w", err) } } - if settings, ok := cfg["user_settings_yaml"]; ok { - res.UserSettingsYaml = settings.(string) + if settings, ok := cfg["user_settings_yaml"].(string); ok && settings != "" { + res.UserSettingsYaml = settings } - if settings, ok := cfg["user_settings_override_yaml"]; ok { - res.UserSettingsOverrideYaml = settings.(string) + if settings, ok := cfg["user_settings_override_yaml"].(string); ok && settings != "" { + res.UserSettingsOverrideYaml = settings } - if v, ok := cfg["docker_image"]; ok { - res.DockerImage = v.(string) + if v, ok := cfg["docker_image"].(string); ok && v != "" { + res.DockerImage = v } } diff --git a/ec/ecresource/deploymentresource/kibana_expanders_test.go b/ec/ecresource/deploymentresource/kibana_expanders_test.go index 2b482b5be..d9b1328e5 100644 --- a/ec/ecresource/deploymentresource/kibana_expanders_test.go +++ b/ec/ecresource/deploymentresource/kibana_expanders_test.go @@ -86,6 +86,45 @@ func Test_expandKibanaResources(t *testing.T) { }, }, }, + { + name: "parses a kibana resource with explicit nils", + args: args{ + tpl: tpl(), + ess: []interface{}{ + map[string]interface{}{ + "ref_id": "main-kibana", + "resource_id": mock.ValidClusterID, + "region": nil, + "elasticsearch_cluster_ref_id": "somerefid", + "topology": []interface{}{map[string]interface{}{ + "instance_configuration_id": "aws.kibana.r5d", + "size": nil, + "zone_count": 1, + }}, + }, + }, + }, + want: []*models.KibanaPayload{ + { + ElasticsearchClusterRefID: ec.String("somerefid"), + Region: ec.String("us-east-1"), + RefID: ec.String("main-kibana"), + Plan: &models.KibanaClusterPlan{ + Kibana: &models.KibanaConfiguration{}, + ClusterTopology: []*models.KibanaClusterTopologyElement{ + { + ZoneCount: 1, + InstanceConfigurationID: "aws.kibana.r5d", + Size: &models.TopologySize{ + Resource: ec.String("memory"), + Value: ec.Int32(1024), + }, + }, + }, + }, + }, + }, + }, { name: "parses a kibana resource with incorrect instance_configuration_id", args: args{ diff --git a/ec/internal/util/parsers.go b/ec/internal/util/parsers.go index 988115e0b..4c512c335 100644 --- a/ec/internal/util/parsers.go +++ b/ec/internal/util/parsers.go @@ -37,23 +37,21 @@ func MemoryToState(mem int32) string { // ParseTopologySize parses a flattened topology into its model. func ParseTopologySize(topology map[string]interface{}) (*models.TopologySize, error) { - if mem, ok := topology["size"]; ok { - if m := mem.(string); m != "" { - val, err := deploymentsize.ParseGb(m) - if err != nil { - return nil, err - } - - var sizeResource = defaultSizeResource - if sr, ok := topology["size_resource"]; ok { - sizeResource = sr.(string) - } - - return &models.TopologySize{ - Value: ec.Int32(val), - Resource: ec.String(sizeResource), - }, nil + if mem, ok := topology["size"].(string); ok && mem != "" { + val, err := deploymentsize.ParseGb(mem) + if err != nil { + return nil, err } + + var sizeResource = defaultSizeResource + if sr, ok := topology["size_resource"].(string); ok { + sizeResource = sr + } + + return &models.TopologySize{ + Value: ec.Int32(val), + Resource: ec.String(sizeResource), + }, nil } return nil, nil From f981d997d7179bebad4dc9d38bbf463bb30403d4 Mon Sep 17 00:00:00 2001 From: Toby Brain Date: Wed, 14 Sep 2022 16:37:51 +1000 Subject: [PATCH 2/8] Ignore fields explicitly set to null in APM resources --- .../deploymentresource/apm_expanders.go | 79 +++++++++---------- .../deploymentresource/apm_expanders_test.go | 41 ++++++++++ 2 files changed, 79 insertions(+), 41 deletions(-) diff --git a/ec/ecresource/deploymentresource/apm_expanders.go b/ec/ecresource/deploymentresource/apm_expanders.go index 14ddbda68..a86654b85 100644 --- a/ec/ecresource/deploymentresource/apm_expanders.go +++ b/ec/ecresource/deploymentresource/apm_expanders.go @@ -53,27 +53,25 @@ func expandApmResources(apms []interface{}, tpl *models.ApmPayload) ([]*models.A func expandApmResource(raw interface{}, res *models.ApmPayload) (*models.ApmPayload, error) { var apm = raw.(map[string]interface{}) - if esRefID, ok := apm["elasticsearch_cluster_ref_id"]; ok { - res.ElasticsearchClusterRefID = ec.String(esRefID.(string)) + if esRefID, ok := apm["elasticsearch_cluster_ref_id"].(string); ok { + res.ElasticsearchClusterRefID = ec.String(esRefID) } - if refID, ok := apm["ref_id"]; ok { - res.RefID = ec.String(refID.(string)) + if refID, ok := apm["ref_id"].(string); ok { + res.RefID = ec.String(refID) } - if region, ok := apm["region"]; ok { - if r := region.(string); r != "" { - res.Region = ec.String(r) - } + if region, ok := apm["region"].(string); ok && region != "" { + res.Region = ec.String(region) } - if cfg, ok := apm["config"]; ok { + if cfg, ok := apm["config"].([]interface{}); ok { if err := expandApmConfig(cfg, res.Plan.Apm); err != nil { return nil, err } } - if rt, ok := apm["topology"]; ok && len(rt.([]interface{})) > 0 { + if rt, ok := apm["topology"].([]interface{}); ok && len(rt) > 0 { topology, err := expandApmTopology(rt, res.Plan.ClusterTopology) if err != nil { return nil, err @@ -86,15 +84,18 @@ func expandApmResource(raw interface{}, res *models.ApmPayload) (*models.ApmPayl return res, nil } -func expandApmTopology(raw interface{}, topologies []*models.ApmTopologyElement) ([]*models.ApmTopologyElement, error) { - rawTopologies := raw.([]interface{}) +func expandApmTopology(rawTopologies []interface{}, topologies []*models.ApmTopologyElement) ([]*models.ApmTopologyElement, error) { res := make([]*models.ApmTopologyElement, 0, len(rawTopologies)) for i, rawTop := range rawTopologies { - topology := rawTop.(map[string]interface{}) + topology, ok := rawTop.(map[string]interface{}) + if !ok { + continue + } + var icID string - if id, ok := topology["instance_configuration_id"]; ok { - icID = id.(string) + if id, ok := topology["instance_configuration_id"].(string); ok { + icID = id } // When a topology element is set but no instance_configuration_id // is set, then obtain the instance_configuration_id from the topology @@ -116,11 +117,8 @@ func expandApmTopology(raw interface{}, topologies []*models.ApmTopologyElement) elem.Size = size } - if zones, ok := topology["zone_count"]; ok { - if z := zones.(int); z > 0 { - elem.ZoneCount = int32(z) - } - + if zones, ok := topology["zone_count"].(int); ok && zones > 0 { + elem.ZoneCount = int32(zones) } res = append(res, elem) @@ -129,40 +127,39 @@ func expandApmTopology(raw interface{}, topologies []*models.ApmTopologyElement) return res, nil } -func expandApmConfig(raw interface{}, res *models.ApmConfiguration) error { - for _, rawCfg := range raw.([]interface{}) { - var cfg = rawCfg.(map[string]interface{}) +func expandApmConfig(raw []interface{}, res *models.ApmConfiguration) error { + for _, rawCfg := range raw { + cfg, ok := rawCfg.(map[string]interface{}) + if !ok { + continue + } - if debugEnabled, ok := cfg["debug_enabled"]; ok { + if debugEnabled, ok := cfg["debug_enabled"].(bool); ok { if res.SystemSettings == nil { res.SystemSettings = &models.ApmSystemSettings{} } - res.SystemSettings.DebugEnabled = ec.Bool(debugEnabled.(bool)) + res.SystemSettings.DebugEnabled = ec.Bool(debugEnabled) } - if settings, ok := cfg["user_settings_json"]; ok && settings != nil { - if s, ok := settings.(string); ok && s != "" { - if err := json.Unmarshal([]byte(s), &res.UserSettingsJSON); err != nil { - return fmt.Errorf("failed expanding apm user_settings_json: %w", err) - } + if settings, ok := cfg["user_settings_json"].(string); ok && settings != "" { + if err := json.Unmarshal([]byte(settings), &res.UserSettingsJSON); err != nil { + return fmt.Errorf("failed expanding apm user_settings_json: %w", err) } } - if settings, ok := cfg["user_settings_override_json"]; ok && settings != nil { - if s, ok := settings.(string); ok && s != "" { - if err := json.Unmarshal([]byte(s), &res.UserSettingsOverrideJSON); err != nil { - return fmt.Errorf("failed expanding apm user_settings_override_json: %w", err) - } + if settings, ok := cfg["user_settings_override_json"].(string); ok && settings != "" { + if err := json.Unmarshal([]byte(settings), &res.UserSettingsOverrideJSON); err != nil { + return fmt.Errorf("failed expanding apm user_settings_override_json: %w", err) } } - if settings, ok := cfg["user_settings_yaml"]; ok { - res.UserSettingsYaml = settings.(string) + if settings, ok := cfg["user_settings_yaml"].(string); ok && settings != "" { + res.UserSettingsYaml = settings } - if settings, ok := cfg["user_settings_override_yaml"]; ok { - res.UserSettingsOverrideYaml = settings.(string) + if settings, ok := cfg["user_settings_override_yaml"].(string); ok && settings != "" { + res.UserSettingsOverrideYaml = settings } - if v, ok := cfg["docker_image"]; ok { - res.DockerImage = v.(string) + if v, ok := cfg["docker_image"].(string); ok { + res.DockerImage = v } } diff --git a/ec/ecresource/deploymentresource/apm_expanders_test.go b/ec/ecresource/deploymentresource/apm_expanders_test.go index 9f0d3b6e3..ec6a00bde 100644 --- a/ec/ecresource/deploymentresource/apm_expanders_test.go +++ b/ec/ecresource/deploymentresource/apm_expanders_test.go @@ -251,6 +251,47 @@ func Test_expandApmResources(t *testing.T) { }, }}, }, + { + name: "parses an APM resource with explicit nils", + args: args{ + tpl: tpl(), + ess: []interface{}{map[string]interface{}{ + "ref_id": "tertiary-apm", + "elasticsearch_cluster_ref_id": "somerefid", + "resource_id": mock.ValidClusterID, + "region": nil, + "config": []interface{}{map[string]interface{}{ + "user_settings_yaml": nil, + "user_settings_override_yaml": nil, + "user_settings_json": nil, + "user_settings_override_json": nil, + "debug_enabled": nil, + }}, + "topology": []interface{}{map[string]interface{}{ + "instance_configuration_id": "aws.apm.r5d", + "size": "4g", + "size_resource": "memory", + "zone_count": 1, + }}, + }}, + }, + want: []*models.ApmPayload{{ + ElasticsearchClusterRefID: ec.String("somerefid"), + Region: ec.String("us-east-1"), + RefID: ec.String("tertiary-apm"), + Plan: &models.ApmPlan{ + Apm: &models.ApmConfiguration{}, + ClusterTopology: []*models.ApmTopologyElement{{ + ZoneCount: 1, + InstanceConfigurationID: "aws.apm.r5d", + Size: &models.TopologySize{ + Resource: ec.String("memory"), + Value: ec.Int32(4096), + }, + }}, + }, + }}, + }, { name: "tries to parse an apm resource when the template doesn't have an APM instance set.", args: args{ From bcb2a7d34a683f06307cbe2352876f03304b72f2 Mon Sep 17 00:00:00 2001 From: Toby Brain Date: Wed, 14 Sep 2022 16:56:46 +1000 Subject: [PATCH 3/8] Ignore fields explicitly set to null in ES resources --- .../elasticsearch_expanders.go | 280 ++++++++---------- .../elasticsearch_expanders_test.go | 109 +++++++ 2 files changed, 240 insertions(+), 149 deletions(-) diff --git a/ec/ecresource/deploymentresource/elasticsearch_expanders.go b/ec/ecresource/deploymentresource/elasticsearch_expanders.go index 67c59ef3d..18506823e 100644 --- a/ec/ecresource/deploymentresource/elasticsearch_expanders.go +++ b/ec/ecresource/deploymentresource/elasticsearch_expanders.go @@ -71,21 +71,19 @@ func expandEsResources(ess []interface{}, tpl *models.ElasticsearchPayload) ([]* func expandEsResource(raw interface{}, res *models.ElasticsearchPayload) (*models.ElasticsearchPayload, error) { es := raw.(map[string]interface{}) - if refID, ok := es["ref_id"]; ok { - res.RefID = ec.String(refID.(string)) + if refID, ok := es["ref_id"].(string); ok { + res.RefID = ec.String(refID) } - if region, ok := es["region"]; ok { - if r := region.(string); r != "" { - res.Region = ec.String(r) - } + if region, ok := es["region"].(string); ok && region != "" { + res.Region = ec.String(region) } // Unsetting the curation properties is since they're deprecated since // >= 6.6.0 which is when ILM is introduced in Elasticsearch. unsetElasticsearchCuration(res) - if rt, ok := es["topology"]; ok && len(rt.([]interface{})) > 0 { + if rt, ok := es["topology"].([]interface{}); ok && len(rt) > 0 { topology, err := expandEsTopology(rt, res.Plan.ClusterTopology) if err != nil { return nil, err @@ -97,75 +95,70 @@ func expandEsResource(raw interface{}, res *models.ElasticsearchPayload) (*model // list when these are set as a dedicated tier as a topology element. updateNodeRolesOnDedicatedTiers(res.Plan.ClusterTopology) - if cfg, ok := es["config"]; ok { + if cfg, ok := es["config"].([]interface{}); ok { if err := expandEsConfig(cfg, res.Plan.Elasticsearch); err != nil { return nil, err } } - if snap, ok := es["snapshot_source"]; ok && len(snap.([]interface{})) > 0 { + if snap, ok := es["snapshot_source"].([]interface{}); ok && len(snap) > 0 { res.Plan.Transient = &models.TransientElasticsearchPlanConfiguration{ RestoreSnapshot: &models.RestoreSnapshotConfiguration{}, } expandSnapshotSource(snap, res.Plan.Transient.RestoreSnapshot) } - if ext, ok := es["extension"]; ok { - if e := ext.(*schema.Set); e.Len() > 0 { - expandEsExtension(e.List(), res.Plan.Elasticsearch) - } + if ext, ok := es["extension"].(*schema.Set); ok && ext.Len() > 0 { + expandEsExtension(ext.List(), res.Plan.Elasticsearch) } - if auto := es["autoscale"]; auto != nil { - if autoscale := auto.(string); autoscale != "" { - autoscaleBool, err := strconv.ParseBool(autoscale) - if err != nil { - return nil, fmt.Errorf("failed parsing autoscale value: %w", err) - } - res.Plan.AutoscalingEnabled = &autoscaleBool + if autoscale, ok := es["autoscale"].(string); ok && autoscale != "" { + autoscaleBool, err := strconv.ParseBool(autoscale) + if err != nil { + return nil, fmt.Errorf("failed parsing autoscale value: %w", err) } + res.Plan.AutoscalingEnabled = &autoscaleBool } - if trust, ok := es["trust_account"]; ok { - if t := trust.(*schema.Set); t.Len() > 0 { - if res.Settings == nil { - res.Settings = &models.ElasticsearchClusterSettings{} - } - expandAccountTrust(t.List(), res.Settings) + if trust, ok := es["trust_account"].(*schema.Set); ok && trust.Len() > 0 { + if res.Settings == nil { + res.Settings = &models.ElasticsearchClusterSettings{} } + expandAccountTrust(trust.List(), res.Settings) } - if trust, ok := es["trust_external"]; ok { - if t := trust.(*schema.Set); t.Len() > 0 { - if res.Settings == nil { - res.Settings = &models.ElasticsearchClusterSettings{} - } - expandExternalTrust(t.List(), res.Settings) + if trust, ok := es["trust_external"].(*schema.Set); ok && trust.Len() > 0 { + if res.Settings == nil { + res.Settings = &models.ElasticsearchClusterSettings{} } + expandExternalTrust(trust.List(), res.Settings) } - if strategy, ok := es["strategy"]; ok { - if s := strategy.([]interface{}); len(s) > 0 { - if res.Plan.Transient == nil { - res.Plan.Transient = &models.TransientElasticsearchPlanConfiguration{ - Strategy: &models.PlanStrategy{}, - } + if strategy, ok := es["strategy"].([]interface{}); ok && len(strategy) > 0 { + if res.Plan.Transient == nil { + res.Plan.Transient = &models.TransientElasticsearchPlanConfiguration{ + Strategy: &models.PlanStrategy{}, } - expandStrategy(s, res.Plan.Transient.Strategy) } + expandStrategy(strategy, res.Plan.Transient.Strategy) } return res, nil } // expandStrategy expands the Configuration Strategy. -func expandStrategy(raw interface{}, strategy *models.PlanStrategy) { - for _, rawStrategy := range raw.([]interface{}) { +func expandStrategy(raw []interface{}, strategy *models.PlanStrategy) { + for _, rawStrategy := range raw { strategyCfg, ok := rawStrategy.(map[string]interface{}) if !ok { continue } - rawValue := strategyCfg["type"].(string) + + rawValue, ok := strategyCfg["type"].(string) + if !ok { + continue + } + if rawValue == autodetect { strategy.Autodetect = new(models.AutodetectStrategyConfig) } else if rawValue == growAndShrink { @@ -181,16 +174,18 @@ func expandStrategy(raw interface{}, strategy *models.PlanStrategy) { } // expandEsTopology expands a flattened topology -func expandEsTopology(raw interface{}, topologies []*models.ElasticsearchClusterTopologyElement) ([]*models.ElasticsearchClusterTopologyElement, error) { - rawTopologies := raw.([]interface{}) +func expandEsTopology(rawTopologies []interface{}, topologies []*models.ElasticsearchClusterTopologyElement) ([]*models.ElasticsearchClusterTopologyElement, error) { res := topologies for _, rawTop := range rawTopologies { - topology := rawTop.(map[string]interface{}) + topology, ok := rawTop.(map[string]interface{}) + if !ok { + continue + } var topologyID string - if id, ok := topology["id"]; ok { - topologyID = id.(string) + if id, ok := topology["id"].(string); ok { + topologyID = id } size, err := util.ParseTopologySize(topology) @@ -206,26 +201,25 @@ func expandEsTopology(raw interface{}, topologies []*models.ElasticsearchCluster elem.Size = size } - if zones, ok := topology["zone_count"]; ok { - if z := zones.(int); z > 0 { - elem.ZoneCount = int32(z) - } + if zones, ok := topology["zone_count"].(int); ok && zones > 0 { + elem.ZoneCount = int32(zones) } if err := parseLegacyNodeType(topology, elem.NodeType); err != nil { return nil, err } - if nr, ok := topology["node_roles"]; ok { - if nrSet, ok := nr.(*schema.Set); ok && nrSet.Len() > 0 { - elem.NodeRoles = util.ItemsToString(nrSet.List()) - elem.NodeType = nil - } + if nrSet, ok := topology["node_roles"].(*schema.Set); ok && nrSet.Len() > 0 { + elem.NodeRoles = util.ItemsToString(nrSet.List()) + elem.NodeType = nil } - if autoscalingRaw := topology["autoscaling"]; autoscalingRaw != nil { - for _, autoscaleRaw := range autoscalingRaw.([]interface{}) { - autoscale := autoscaleRaw.(map[string]interface{}) + if autoscalingRaw, ok := topology["autoscaling"].([]interface{}); ok && len(autoscalingRaw) > 0 { + for _, autoscaleRaw := range autoscalingRaw { + autoscale, ok := autoscaleRaw.(map[string]interface{}) + if !ok { + continue + } if elem.AutoscalingMax == nil { elem.AutoscalingMax = new(models.TopologySize) @@ -253,22 +247,20 @@ func expandEsTopology(raw interface{}, topologies []*models.ElasticsearchCluster elem.AutoscalingMax = nil } - if policy := autoscale["policy_override_json"]; policy != nil { - if policyString := policy.(string); policyString != "" { - if err := json.Unmarshal([]byte(policyString), - &elem.AutoscalingPolicyOverrideJSON, - ); err != nil { - return nil, fmt.Errorf( - "elasticsearch topology %s: unable to load policy_override_json: %w", - topologyID, err, - ) - } + if policy, ok := autoscale["policy_override_json"].(string); ok && policy != "" { + if err := json.Unmarshal([]byte(policy), + &elem.AutoscalingPolicyOverrideJSON, + ); err != nil { + return nil, fmt.Errorf( + "elasticsearch topology %s: unable to load policy_override_json: %w", + topologyID, err, + ) } } } } - if cfg, ok := topology["config"]; ok { + if cfg, ok := topology["config"].([]interface{}); ok { if elem.Elasticsearch == nil { elem.Elasticsearch = &models.ElasticsearchConfiguration{} } @@ -290,81 +282,77 @@ func expandAutoscalingDimension(autoscale map[string]interface{}, model *models. sizeAttribute := fmt.Sprintf("%s_size", dimension) resourceAttribute := fmt.Sprintf("%s_size_resource", dimension) - if size := autoscale[sizeAttribute]; size != nil { - if size := size.(string); size != "" { - val, err := deploymentsize.ParseGb(size) - if err != nil { - return err - } - model.Value = &val + if size, ok := autoscale[sizeAttribute].(string); ok && size != "" { + val, err := deploymentsize.ParseGb(size) + if err != nil { + return err + } + model.Value = &val - if model.Resource == nil { - model.Resource = ec.String("memory") - } + if model.Resource == nil { + model.Resource = ec.String("memory") } } - if sizeResource := autoscale[resourceAttribute]; sizeResource != nil { - if sizeResource := sizeResource.(string); sizeResource != "" { - model.Resource = ec.String(sizeResource) - } + if sizeResource, ok := autoscale[resourceAttribute].(string); ok && sizeResource != "" { + model.Resource = ec.String(sizeResource) } return nil } -func expandEsConfig(raw interface{}, esCfg *models.ElasticsearchConfiguration) error { - for _, rawCfg := range raw.([]interface{}) { +func expandEsConfig(raw []interface{}, esCfg *models.ElasticsearchConfiguration) error { + for _, rawCfg := range raw { cfg, ok := rawCfg.(map[string]interface{}) if !ok { continue } - if settings, ok := cfg["user_settings_json"]; ok && settings != nil { - if s, ok := settings.(string); ok && s != "" { - if err := json.Unmarshal([]byte(s), &esCfg.UserSettingsJSON); err != nil { - return fmt.Errorf( - "failed expanding elasticsearch user_settings_json: %w", err, - ) - } + if settings, ok := cfg["user_settings_json"].(string); ok && settings != "" { + if err := json.Unmarshal([]byte(settings), &esCfg.UserSettingsJSON); err != nil { + return fmt.Errorf( + "failed expanding elasticsearch user_settings_json: %w", err, + ) } } - if settings, ok := cfg["user_settings_override_json"]; ok && settings != nil { - if s, ok := settings.(string); ok && s != "" { - if err := json.Unmarshal([]byte(s), &esCfg.UserSettingsOverrideJSON); err != nil { - return fmt.Errorf( - "failed expanding elasticsearch user_settings_override_json: %w", err, - ) - } + if settings, ok := cfg["user_settings_override_json"].(string); ok && settings != "" { + if err := json.Unmarshal([]byte(settings), &esCfg.UserSettingsOverrideJSON); err != nil { + return fmt.Errorf( + "failed expanding elasticsearch user_settings_override_json: %w", err, + ) } } - if settings, ok := cfg["user_settings_yaml"]; ok { - esCfg.UserSettingsYaml = settings.(string) + if settings, ok := cfg["user_settings_yaml"].(string); ok && settings != "" { + esCfg.UserSettingsYaml = settings } - if settings, ok := cfg["user_settings_override_yaml"]; ok { - esCfg.UserSettingsOverrideYaml = settings.(string) + if settings, ok := cfg["user_settings_override_yaml"].(string); ok && settings != "" { + esCfg.UserSettingsOverrideYaml = settings } - if v, ok := cfg["plugins"]; ok { - esCfg.EnabledBuiltInPlugins = util.ItemsToString(v.(*schema.Set).List()) + if v, ok := cfg["plugins"].(*schema.Set); ok && v.Len() > 0 { + esCfg.EnabledBuiltInPlugins = util.ItemsToString(v.List()) } - if v, ok := cfg["docker_image"]; ok { - esCfg.DockerImage = v.(string) + if v, ok := cfg["docker_image"].(string); ok { + esCfg.DockerImage = v } } return nil } -func expandSnapshotSource(raw interface{}, restore *models.RestoreSnapshotConfiguration) { - for _, rawRestore := range raw.([]interface{}) { - var rs = rawRestore.(map[string]interface{}) - if clusterID, ok := rs["source_elasticsearch_cluster_id"]; ok { - restore.SourceClusterID = clusterID.(string) +func expandSnapshotSource(raw []interface{}, restore *models.RestoreSnapshotConfiguration) { + for _, rawRestore := range raw { + var rs, ok = rawRestore.(map[string]interface{}) + if !ok { + continue } - if snapshotName, ok := rs["snapshot_name"]; ok { - restore.SnapshotName = ec.String(snapshotName.(string)) + if clusterID, ok := rs["source_elasticsearch_cluster_id"].(string); ok { + restore.SourceClusterID = clusterID + } + + if snapshotName, ok := rs["snapshot_name"].(string); ok { + restore.SnapshotName = ec.String(snapshotName) } } } @@ -428,32 +416,32 @@ func parseLegacyNodeType(topology map[string]interface{}, nodeType *models.Elast return nil } - if ntData, ok := topology["node_type_data"]; ok && ntData.(string) != "" { - nt, err := strconv.ParseBool(ntData.(string)) + if ntData, ok := topology["node_type_data"].(string); ok && ntData != "" { + nt, err := strconv.ParseBool(ntData) if err != nil { return fmt.Errorf("failed parsing node_type_data value: %w", err) } nodeType.Data = ec.Bool(nt) } - if ntMaster, ok := topology["node_type_master"]; ok && ntMaster.(string) != "" { - nt, err := strconv.ParseBool(ntMaster.(string)) + if ntMaster, ok := topology["node_type_master"].(string); ok && ntMaster != "" { + nt, err := strconv.ParseBool(ntMaster) if err != nil { return fmt.Errorf("failed parsing node_type_master value: %w", err) } nodeType.Master = ec.Bool(nt) } - if ntIngest, ok := topology["node_type_ingest"]; ok && ntIngest.(string) != "" { - nt, err := strconv.ParseBool(ntIngest.(string)) + if ntIngest, ok := topology["node_type_ingest"].(string); ok && ntIngest != "" { + nt, err := strconv.ParseBool(ntIngest) if err != nil { return fmt.Errorf("failed parsing node_type_ingest value: %w", err) } nodeType.Ingest = ec.Bool(nt) } - if ntMl, ok := topology["node_type_ml"]; ok && ntMl.(string) != "" { - nt, err := strconv.ParseBool(ntMl.(string)) + if ntMl, ok := topology["node_type_ml"].(string); ok && ntMl != "" { + nt, err := strconv.ParseBool(ntMl) if err != nil { return fmt.Errorf("failed parsing node_type_ml value: %w", err) } @@ -539,21 +527,21 @@ func expandEsExtension(raw []interface{}, es *models.ElasticsearchConfiguration) m := rawExt.(map[string]interface{}) var version string - if v, ok := m["version"]; ok { - version = v.(string) + if v, ok := m["version"].(string); ok { + version = v } var url string - if u, ok := m["url"]; ok { - url = u.(string) + if u, ok := m["url"].(string); ok { + url = u } var name string - if n, ok := m["name"]; ok { - name = n.(string) + if n, ok := m["name"].(string); ok { + name = n } - if t, ok := m["type"]; ok && t.(string) == "bundle" { + if t, ok := m["type"].(string); ok && t == "bundle" { es.UserBundles = append(es.UserBundles, &models.ElasticsearchUserBundle{ Name: &name, ElasticsearchVersion: &version, @@ -561,7 +549,7 @@ func expandEsExtension(raw []interface{}, es *models.ElasticsearchConfiguration) }) } - if t, ok := m["type"]; ok && t.(string) == "plugin" { + if t, ok := m["type"].(string); ok && t == "plugin" { es.UserPlugins = append(es.UserPlugins, &models.ElasticsearchUserPlugin{ Name: &name, ElasticsearchVersion: &version, @@ -577,21 +565,18 @@ func expandAccountTrust(raw []interface{}, es *models.ElasticsearchClusterSettin m := rawTrust.(map[string]interface{}) var id string - if v, ok := m["account_id"]; ok { - id = v.(string) + if v, ok := m["account_id"].(string); ok { + id = v } var all bool - if a, ok := m["trust_all"]; ok { - all = a.(bool) + if a, ok := m["trust_all"].(bool); ok { + all = a } var allowlist []string - if al, ok := m["trust_allowlist"]; ok { - set := al.(*schema.Set) - if set.Len() > 0 { - allowlist = util.ItemsToString(set.List()) - } + if al, ok := m["trust_allowlist"].(*schema.Set); ok && al.Len() > 0 { + allowlist = util.ItemsToString(al.List()) } accounts = append(accounts, &models.AccountTrustRelationship{ @@ -618,21 +603,18 @@ func expandExternalTrust(raw []interface{}, es *models.ElasticsearchClusterSetti m := rawTrust.(map[string]interface{}) var id string - if v, ok := m["relationship_id"]; ok { - id = v.(string) + if v, ok := m["relationship_id"].(string); ok { + id = v } var all bool - if a, ok := m["trust_all"]; ok { - all = a.(bool) + if a, ok := m["trust_all"].(bool); ok { + all = a } var allowlist []string - if al, ok := m["trust_allowlist"]; ok { - set := al.(*schema.Set) - if set.Len() > 0 { - allowlist = util.ItemsToString(set.List()) - } + if al, ok := m["trust_allowlist"].(*schema.Set); ok && al.Len() > 0 { + allowlist = util.ItemsToString(al.List()) } external = append(external, &models.ExternalTrustRelationship{ diff --git a/ec/ecresource/deploymentresource/elasticsearch_expanders_test.go b/ec/ecresource/deploymentresource/elasticsearch_expanders_test.go index b30edf8c8..26a473c30 100644 --- a/ec/ecresource/deploymentresource/elasticsearch_expanders_test.go +++ b/ec/ecresource/deploymentresource/elasticsearch_expanders_test.go @@ -593,6 +593,115 @@ func Test_expandEsResource(t *testing.T) { }, }), }, + { + name: "parses an ES resource with explicit nils", + args: args{ + dt: hotWarmTpl770(), + ess: []interface{}{ + map[string]interface{}{ + "ref_id": "main-elasticsearch", + "resource_id": mock.ValidClusterID, + "version": "7.7.0", + "region": "some-region", + "deployment_template_id": "aws-hot-warm-v2", + "config": []interface{}{map[string]interface{}{ + "user_settings_yaml": nil, + }}, + "topology": []interface{}{ + map[string]interface{}{ + "id": "hot_content", + "size": nil, + "zone_count": 1, + }, + map[string]interface{}{ + "id": "warm", + "size": "2g", + "zone_count": nil, + }, + }, + }, + }, + }, + want: enrichWithEmptyTopologies(hotWarmTpl770(), &models.ElasticsearchPayload{ + Region: ec.String("some-region"), + RefID: ec.String("main-elasticsearch"), + Settings: &models.ElasticsearchClusterSettings{ + DedicatedMastersThreshold: 6, + Curation: nil, + }, + Plan: &models.ElasticsearchClusterPlan{ + AutoscalingEnabled: ec.Bool(false), + Elasticsearch: &models.ElasticsearchConfiguration{ + Version: "7.7.0", + Curation: nil, + UserSettingsYaml: "", + }, + DeploymentTemplate: &models.DeploymentTemplateReference{ + ID: ec.String("aws-hot-warm-v2"), + }, + ClusterTopology: []*models.ElasticsearchClusterTopologyElement{ + { + ID: "hot_content", + Elasticsearch: &models.ElasticsearchConfiguration{ + NodeAttributes: map[string]string{ + "data": "hot", + }, + }, + ZoneCount: 1, + InstanceConfigurationID: "aws.data.highio.i3", + Size: &models.TopologySize{ + Resource: ec.String("memory"), + Value: ec.Int32(4096), + }, + NodeType: &models.ElasticsearchNodeType{ + Data: ec.Bool(true), + Ingest: ec.Bool(true), + Master: ec.Bool(true), + }, + TopologyElementControl: &models.TopologyElementControl{ + Min: &models.TopologySize{ + Resource: ec.String("memory"), + Value: ec.Int32(1024), + }, + }, + AutoscalingMax: &models.TopologySize{ + Value: ec.Int32(118784), + Resource: ec.String("memory"), + }, + }, + { + ID: "warm", + Elasticsearch: &models.ElasticsearchConfiguration{ + NodeAttributes: map[string]string{ + "data": "warm", + }, + }, + ZoneCount: 2, + InstanceConfigurationID: "aws.data.highstorage.d2", + Size: &models.TopologySize{ + Resource: ec.String("memory"), + Value: ec.Int32(2048), + }, + NodeType: &models.ElasticsearchNodeType{ + Data: ec.Bool(true), + Ingest: ec.Bool(true), + Master: ec.Bool(false), + }, + TopologyElementControl: &models.TopologyElementControl{ + Min: &models.TopologySize{ + Resource: ec.String("memory"), + Value: ec.Int32(0), + }, + }, + AutoscalingMax: &models.TopologySize{ + Value: ec.Int32(118784), + Resource: ec.String("memory"), + }, + }, + }, + }, + }), + }, { name: "parses an ES resource without a topology (HotWarm)", args: args{ From b6ef317a3535c8adddb1cec5aed8fa7b228fa41f Mon Sep 17 00:00:00 2001 From: Toby Brain Date: Wed, 14 Sep 2022 17:02:39 +1000 Subject: [PATCH 4/8] Ignore fields explicitly set to null in Enterprise Search resources --- .../enterprise_search_expanders.go | 79 +++++++++---------- .../enterprise_search_expanders_test.go | 43 ++++++++++ 2 files changed, 82 insertions(+), 40 deletions(-) diff --git a/ec/ecresource/deploymentresource/enterprise_search_expanders.go b/ec/ecresource/deploymentresource/enterprise_search_expanders.go index 96e32a110..68a559443 100644 --- a/ec/ecresource/deploymentresource/enterprise_search_expanders.go +++ b/ec/ecresource/deploymentresource/enterprise_search_expanders.go @@ -53,31 +53,29 @@ func expandEssResources(ess []interface{}, tpl *models.EnterpriseSearchPayload) func expandEssResource(raw interface{}, res *models.EnterpriseSearchPayload) (*models.EnterpriseSearchPayload, error) { ess := raw.(map[string]interface{}) - if esRefID, ok := ess["elasticsearch_cluster_ref_id"]; ok { - res.ElasticsearchClusterRefID = ec.String(esRefID.(string)) + if esRefID, ok := ess["elasticsearch_cluster_ref_id"].(string); ok { + res.ElasticsearchClusterRefID = ec.String(esRefID) } - if refID, ok := ess["ref_id"]; ok { - res.RefID = ec.String(refID.(string)) + if refID, ok := ess["ref_id"].(string); ok { + res.RefID = ec.String(refID) } - if version, ok := ess["version"]; ok { - res.Plan.EnterpriseSearch.Version = version.(string) + if version, ok := ess["version"].(string); ok { + res.Plan.EnterpriseSearch.Version = version } - if region, ok := ess["region"]; ok { - if r := region.(string); r != "" { - res.Region = ec.String(r) - } + if region, ok := ess["region"].(string); ok && region != "" { + res.Region = ec.String(region) } - if cfg, ok := ess["config"]; ok { + if cfg, ok := ess["config"].([]interface{}); ok { if err := expandEssConfig(cfg, res.Plan.EnterpriseSearch); err != nil { return nil, err } } - if rt, ok := ess["topology"]; ok && len(rt.([]interface{})) > 0 { + if rt, ok := ess["topology"].([]interface{}); ok && len(rt) > 0 { topology, err := expandEssTopology(rt, res.Plan.ClusterTopology) if err != nil { return nil, err @@ -90,14 +88,17 @@ func expandEssResource(raw interface{}, res *models.EnterpriseSearchPayload) (*m return res, nil } -func expandEssTopology(raw interface{}, topologies []*models.EnterpriseSearchTopologyElement) ([]*models.EnterpriseSearchTopologyElement, error) { - rawTopologies := raw.([]interface{}) +func expandEssTopology(rawTopologies []interface{}, topologies []*models.EnterpriseSearchTopologyElement) ([]*models.EnterpriseSearchTopologyElement, error) { res := make([]*models.EnterpriseSearchTopologyElement, 0, len(rawTopologies)) for i, rawTop := range rawTopologies { - topology := rawTop.(map[string]interface{}) + topology, ok := rawTop.(map[string]interface{}) + if !ok { + continue + } + var icID string - if id, ok := topology["instance_configuration_id"]; ok { - icID = id.(string) + if id, ok := topology["instance_configuration_id"].(string); ok { + icID = id } // When a topology element is set but no instance_configuration_id @@ -129,10 +130,8 @@ func expandEssTopology(raw interface{}, topologies []*models.EnterpriseSearchTop elem.Size = size } - if zones, ok := topology["zone_count"]; ok { - if z := zones.(int); z > 0 { - elem.ZoneCount = int32(z) - } + if zones, ok := topology["zone_count"].(int); ok && zones > 0 { + elem.ZoneCount = int32(zones) } res = append(res, elem) @@ -141,32 +140,32 @@ func expandEssTopology(raw interface{}, topologies []*models.EnterpriseSearchTop return res, nil } -func expandEssConfig(raw interface{}, res *models.EnterpriseSearchConfiguration) error { - for _, rawCfg := range raw.([]interface{}) { - cfg := rawCfg.(map[string]interface{}) - if settings, ok := cfg["user_settings_json"]; ok && settings != nil { - if s, ok := settings.(string); ok && s != "" { - if err := json.Unmarshal([]byte(s), &res.UserSettingsJSON); err != nil { - return fmt.Errorf("failed expanding enterprise_search user_settings_json: %w", err) - } +func expandEssConfig(raw []interface{}, res *models.EnterpriseSearchConfiguration) error { + for _, rawCfg := range raw { + cfg, ok := rawCfg.(map[string]interface{}) + if !ok { + continue + } + + if settings, ok := cfg["user_settings_json"].(string); ok && settings != "" { + if err := json.Unmarshal([]byte(settings), &res.UserSettingsJSON); err != nil { + return fmt.Errorf("failed expanding enterprise_search user_settings_json: %w", err) } } - if settings, ok := cfg["user_settings_override_json"]; ok && settings != nil { - if s, ok := settings.(string); ok && s != "" { - if err := json.Unmarshal([]byte(s), &res.UserSettingsOverrideJSON); err != nil { - return fmt.Errorf("failed expanding enterprise_search user_settings_override_json: %w", err) - } + if settings, ok := cfg["user_settings_override_json"].(string); ok && settings != "" { + if err := json.Unmarshal([]byte(settings), &res.UserSettingsOverrideJSON); err != nil { + return fmt.Errorf("failed expanding enterprise_search user_settings_override_json: %w", err) } } - if settings, ok := cfg["user_settings_yaml"]; ok { - res.UserSettingsYaml = settings.(string) + if settings, ok := cfg["user_settings_yaml"].(string); ok && settings != "" { + res.UserSettingsYaml = settings } - if settings, ok := cfg["user_settings_override_yaml"]; ok { - res.UserSettingsOverrideYaml = settings.(string) + if settings, ok := cfg["user_settings_override_yaml"].(string); ok && settings != "" { + res.UserSettingsOverrideYaml = settings } - if v, ok := cfg["docker_image"]; ok { - res.DockerImage = v.(string) + if v, ok := cfg["docker_image"].(string); ok { + res.DockerImage = v } } diff --git a/ec/ecresource/deploymentresource/enterprise_search_expanders_test.go b/ec/ecresource/deploymentresource/enterprise_search_expanders_test.go index 2db79a4b6..021d50dd0 100644 --- a/ec/ecresource/deploymentresource/enterprise_search_expanders_test.go +++ b/ec/ecresource/deploymentresource/enterprise_search_expanders_test.go @@ -319,6 +319,49 @@ func Test_expandEssResources(t *testing.T) { }, }}, }, + { + name: "parses an enterprise_search resource with explicit nils", + args: args{ + tpl: tpl(), + ess: []interface{}{map[string]interface{}{ + "ref_id": "secondary-enterprise_search", + "elasticsearch_cluster_ref_id": "somerefid", + "resource_id": mock.ValidClusterID, + "version": "7.7.0", + "region": nil, + "config": []interface{}{map[string]interface{}{ + "user_settings_yaml": nil, + "user_settings_override_yaml": nil, + "user_settings_json": nil, + "user_settings_override_json": nil, + }}, + "topology": nil, + }}, + }, + want: []*models.EnterpriseSearchPayload{{ + ElasticsearchClusterRefID: ec.String("somerefid"), + Region: ec.String("us-east-1"), + RefID: ec.String("secondary-enterprise_search"), + Plan: &models.EnterpriseSearchPlan{ + EnterpriseSearch: &models.EnterpriseSearchConfiguration{ + Version: "7.7.0", + }, + ClusterTopology: []*models.EnterpriseSearchTopologyElement{{ + ZoneCount: 2, + InstanceConfigurationID: "aws.enterprisesearch.m5d", + Size: &models.TopologySize{ + Resource: ec.String("memory"), + Value: ec.Int32(2048), + }, + NodeType: &models.EnterpriseSearchNodeTypes{ + Appserver: ec.Bool(true), + Connector: ec.Bool(true), + Worker: ec.Bool(true), + }, + }}, + }, + }}, + }, { name: "parses an enterprise_search resource with invalid instance_configuration_id", args: args{ From f6b99942b58145e089db0601c7b616845e590864 Mon Sep 17 00:00:00 2001 From: Toby Brain Date: Wed, 14 Sep 2022 17:09:56 +1000 Subject: [PATCH 5/8] Ignore fields explicitly set to null in Integrations Server resources --- .../integrations_server_expanders.go | 89 +++++++++---------- .../integrations_server_expanders_test.go | 45 ++++++++++ 2 files changed, 88 insertions(+), 46 deletions(-) diff --git a/ec/ecresource/deploymentresource/integrations_server_expanders.go b/ec/ecresource/deploymentresource/integrations_server_expanders.go index ff98249a4..d38fc64f0 100644 --- a/ec/ecresource/deploymentresource/integrations_server_expanders.go +++ b/ec/ecresource/deploymentresource/integrations_server_expanders.go @@ -29,8 +29,8 @@ import ( ) // expandIntegrationsServerResources expands IntegrationsServer resources into their models. -func expandIntegrationsServerResources(IntegrationsServers []interface{}, tpl *models.IntegrationsServerPayload) ([]*models.IntegrationsServerPayload, error) { - if len(IntegrationsServers) == 0 { +func expandIntegrationsServerResources(integrationsServers []interface{}, tpl *models.IntegrationsServerPayload) ([]*models.IntegrationsServerPayload, error) { + if len(integrationsServers) == 0 { return nil, nil } @@ -38,8 +38,8 @@ func expandIntegrationsServerResources(IntegrationsServers []interface{}, tpl *m return nil, errors.New("IntegrationsServer specified but deployment template is not configured for it. Use a different template if you wish to add IntegrationsServer") } - result := make([]*models.IntegrationsServerPayload, 0, len(IntegrationsServers)) - for _, raw := range IntegrationsServers { + result := make([]*models.IntegrationsServerPayload, 0, len(integrationsServers)) + for _, raw := range integrationsServers { resResource, err := expandIntegrationsServerResource(raw, tpl) if err != nil { return nil, err @@ -51,29 +51,27 @@ func expandIntegrationsServerResources(IntegrationsServers []interface{}, tpl *m } func expandIntegrationsServerResource(raw interface{}, res *models.IntegrationsServerPayload) (*models.IntegrationsServerPayload, error) { - var IntegrationsServer = raw.(map[string]interface{}) + var integrationsServer = raw.(map[string]interface{}) - if esRefID, ok := IntegrationsServer["elasticsearch_cluster_ref_id"]; ok { - res.ElasticsearchClusterRefID = ec.String(esRefID.(string)) + if esRefID, ok := integrationsServer["elasticsearch_cluster_ref_id"].(string); ok { + res.ElasticsearchClusterRefID = ec.String(esRefID) } - if refID, ok := IntegrationsServer["ref_id"]; ok { - res.RefID = ec.String(refID.(string)) + if refID, ok := integrationsServer["ref_id"].(string); ok { + res.RefID = ec.String(refID) } - if region, ok := IntegrationsServer["region"]; ok { - if r := region.(string); r != "" { - res.Region = ec.String(r) - } + if region, ok := integrationsServer["region"].(string); ok && region != "" { + res.Region = ec.String(region) } - if cfg, ok := IntegrationsServer["config"]; ok { + if cfg, ok := integrationsServer["config"].([]interface{}); ok { if err := expandIntegrationsServerConfig(cfg, res.Plan.IntegrationsServer); err != nil { return nil, err } } - if rt, ok := IntegrationsServer["topology"]; ok && len(rt.([]interface{})) > 0 { + if rt, ok := integrationsServer["topology"].([]interface{}); ok && len(rt) > 0 { topology, err := expandIntegrationsServerTopology(rt, res.Plan.ClusterTopology) if err != nil { return nil, err @@ -86,15 +84,18 @@ func expandIntegrationsServerResource(raw interface{}, res *models.IntegrationsS return res, nil } -func expandIntegrationsServerTopology(raw interface{}, topologies []*models.IntegrationsServerTopologyElement) ([]*models.IntegrationsServerTopologyElement, error) { - rawTopologies := raw.([]interface{}) +func expandIntegrationsServerTopology(rawTopologies []interface{}, topologies []*models.IntegrationsServerTopologyElement) ([]*models.IntegrationsServerTopologyElement, error) { res := make([]*models.IntegrationsServerTopologyElement, 0, len(rawTopologies)) for i, rawTop := range rawTopologies { - topology := rawTop.(map[string]interface{}) + topology, ok := rawTop.(map[string]interface{}) + if !ok { + continue + } + var icID string - if id, ok := topology["instance_configuration_id"]; ok { - icID = id.(string) + if id, ok := topology["instance_configuration_id"].(string); ok { + icID = id } // When a topology element is set but no instance_configuration_id // is set, then obtain the instance_configuration_id from the topology @@ -116,11 +117,8 @@ func expandIntegrationsServerTopology(raw interface{}, topologies []*models.Inte elem.Size = size } - if zones, ok := topology["zone_count"]; ok { - if z := zones.(int); z > 0 { - elem.ZoneCount = int32(z) - } - + if zones, ok := topology["zone_count"].(int); ok && zones > 0 { + elem.ZoneCount = int32(zones) } res = append(res, elem) @@ -129,40 +127,39 @@ func expandIntegrationsServerTopology(raw interface{}, topologies []*models.Inte return res, nil } -func expandIntegrationsServerConfig(raw interface{}, res *models.IntegrationsServerConfiguration) error { - for _, rawCfg := range raw.([]interface{}) { - var cfg = rawCfg.(map[string]interface{}) +func expandIntegrationsServerConfig(raw []interface{}, res *models.IntegrationsServerConfiguration) error { + for _, rawCfg := range raw { + cfg, ok := rawCfg.(map[string]interface{}) + if !ok { + continue + } - if debugEnabled, ok := cfg["debug_enabled"]; ok { + if debugEnabled, ok := cfg["debug_enabled"].(bool); ok { if res.SystemSettings == nil { res.SystemSettings = &models.IntegrationsServerSystemSettings{} } - res.SystemSettings.DebugEnabled = ec.Bool(debugEnabled.(bool)) + res.SystemSettings.DebugEnabled = ec.Bool(debugEnabled) } - if settings, ok := cfg["user_settings_json"]; ok && settings != nil { - if s, ok := settings.(string); ok && s != "" { - if err := json.Unmarshal([]byte(s), &res.UserSettingsJSON); err != nil { - return fmt.Errorf("failed expanding IntegrationsServer user_settings_json: %w", err) - } + if settings, ok := cfg["user_settings_json"].(string); ok && settings != "" { + if err := json.Unmarshal([]byte(settings), &res.UserSettingsJSON); err != nil { + return fmt.Errorf("failed expanding IntegrationsServer user_settings_json: %w", err) } } - if settings, ok := cfg["user_settings_override_json"]; ok && settings != nil { - if s, ok := settings.(string); ok && s != "" { - if err := json.Unmarshal([]byte(s), &res.UserSettingsOverrideJSON); err != nil { - return fmt.Errorf("failed expanding IntegrationsServer user_settings_override_json: %w", err) - } + if settings, ok := cfg["user_settings_override_json"].(string); ok && settings != "" { + if err := json.Unmarshal([]byte(settings), &res.UserSettingsOverrideJSON); err != nil { + return fmt.Errorf("failed expanding IntegrationsServer user_settings_override_json: %w", err) } } - if settings, ok := cfg["user_settings_yaml"]; ok { - res.UserSettingsYaml = settings.(string) + if settings, ok := cfg["user_settings_yaml"].(string); ok && settings != "" { + res.UserSettingsYaml = settings } - if settings, ok := cfg["user_settings_override_yaml"]; ok { - res.UserSettingsOverrideYaml = settings.(string) + if settings, ok := cfg["user_settings_override_yaml"].(string); ok && settings != "" { + res.UserSettingsOverrideYaml = settings } - if v, ok := cfg["docker_image"]; ok { - res.DockerImage = v.(string) + if v, ok := cfg["docker_image"].(string); ok { + res.DockerImage = v } } diff --git a/ec/ecresource/deploymentresource/integrations_server_expanders_test.go b/ec/ecresource/deploymentresource/integrations_server_expanders_test.go index 288029afa..263189ed3 100644 --- a/ec/ecresource/deploymentresource/integrations_server_expanders_test.go +++ b/ec/ecresource/deploymentresource/integrations_server_expanders_test.go @@ -251,6 +251,51 @@ func Test_expandIntegrationsServerResources(t *testing.T) { }, }}, }, + { + name: "parses an Integrations Server resource with explicit nils", + args: args{ + tpl: tpl(), + ess: []interface{}{map[string]interface{}{ + "ref_id": "tertiary-integrations_server", + "elasticsearch_cluster_ref_id": "somerefid", + "resource_id": mock.ValidClusterID, + "region": "some-region", + "config": []interface{}{map[string]interface{}{ + "user_settings_yaml": nil, + "user_settings_override_yaml": nil, + "user_settings_json": nil, + "user_settings_override_json": nil, + "debug_enabled": true, + }}, + "topology": []interface{}{map[string]interface{}{ + "instance_configuration_id": "integrations.server", + "size": nil, + "size_resource": "memory", + "zone_count": 1, + }}, + }}, + }, + want: []*models.IntegrationsServerPayload{{ + ElasticsearchClusterRefID: ec.String("somerefid"), + Region: ec.String("some-region"), + RefID: ec.String("tertiary-integrations_server"), + Plan: &models.IntegrationsServerPlan{ + IntegrationsServer: &models.IntegrationsServerConfiguration{ + SystemSettings: &models.IntegrationsServerSystemSettings{ + DebugEnabled: ec.Bool(true), + }, + }, + ClusterTopology: []*models.IntegrationsServerTopologyElement{{ + ZoneCount: 1, + InstanceConfigurationID: "integrations.server", + Size: &models.TopologySize{ + Resource: ec.String("memory"), + Value: ec.Int32(1024), + }, + }}, + }, + }}, + }, { name: "tries to parse an integrations_server resource when the template doesn't have an Integrations Server instance set.", args: args{ From c6597c698c2d54e8f56448ce73ed72d9276fc60f Mon Sep 17 00:00:00 2001 From: Toby Brain Date: Wed, 14 Sep 2022 17:17:10 +1000 Subject: [PATCH 6/8] Ignore fields explicitly set to null in observability resources --- .../deploymentresource/observability.go | 18 +++++++++--------- .../deploymentresource/observability_test.go | 12 ++++++++++++ 2 files changed, 21 insertions(+), 9 deletions(-) diff --git a/ec/ecresource/deploymentresource/observability.go b/ec/ecresource/deploymentresource/observability.go index 34b34e82f..5442fc116 100644 --- a/ec/ecresource/deploymentresource/observability.go +++ b/ec/ecresource/deploymentresource/observability.go @@ -66,12 +66,12 @@ func expandObservability(raw []interface{}, client *api.API) (*models.Deployment for _, rawObs := range raw { var obs = rawObs.(map[string]interface{}) - depID, ok := obs["deployment_id"] + depID, ok := obs["deployment_id"].(string) if !ok { return nil, nil } - refID, ok := obs["ref_id"] + refID, ok := obs["ref_id"].(string) if depID == "self" { // For self monitoring, the refID is not mandatory if !ok { @@ -83,7 +83,7 @@ func expandObservability(raw []interface{}, client *api.API) (*models.Deployment params := deploymentapi.PopulateRefIDParams{ Kind: util.Elasticsearch, API: client, - DeploymentID: depID.(string), + DeploymentID: depID, RefID: ec.String(""), } @@ -94,20 +94,20 @@ func expandObservability(raw []interface{}, client *api.API) (*models.Deployment refID = *params.RefID } - if logging := obs["logs"]; logging.(bool) { + if logging, ok := obs["logs"].(bool); ok && logging { req.Logging = &models.DeploymentLoggingSettings{ Destination: &models.ObservabilityAbsoluteDeployment{ - DeploymentID: ec.String(depID.(string)), - RefID: refID.(string), + DeploymentID: ec.String(depID), + RefID: refID, }, } } - if metrics := obs["metrics"]; metrics.(bool) { + if metrics, ok := obs["metrics"].(bool); ok && metrics { req.Metrics = &models.DeploymentMetricsSettings{ Destination: &models.ObservabilityAbsoluteDeployment{ - DeploymentID: ec.String(depID.(string)), - RefID: refID.(string), + DeploymentID: ec.String(depID), + RefID: refID, }, } } diff --git a/ec/ecresource/deploymentresource/observability_test.go b/ec/ecresource/deploymentresource/observability_test.go index 96ab053fb..3fcf482a0 100644 --- a/ec/ecresource/deploymentresource/observability_test.go +++ b/ec/ecresource/deploymentresource/observability_test.go @@ -157,6 +157,18 @@ func TestExpandObservability(t *testing.T) { }, }, }, + { + name: "handles explicit nils", + args: args{ + v: []interface{}{map[string]interface{}{ + "deployment_id": mock.ValidClusterID, + "ref_id": "main-elasticsearch", + "metrics": nil, + "logs": nil, + }}, + }, + want: &models.DeploymentObservabilitySettings{}, + }, { name: "expands all observability settings", args: args{ From 3eed065bbcdb7998c1c8f914bd16df2fccf58d89 Mon Sep 17 00:00:00 2001 From: Toby Brain Date: Wed, 14 Sep 2022 17:23:19 +1000 Subject: [PATCH 7/8] Ignore fields explicitly set to null in azure private endpoint resources --- .../trafficfilterresource/expanders.go | 20 ++++++++----- .../trafficfilterresource/expanders_test.go | 30 +++++++++++++++++++ 2 files changed, 42 insertions(+), 8 deletions(-) diff --git a/ec/ecresource/trafficfilterresource/expanders.go b/ec/ecresource/trafficfilterresource/expanders.go index 9e7d7eb8d..8c696ee30 100644 --- a/ec/ecresource/trafficfilterresource/expanders.go +++ b/ec/ecresource/trafficfilterresource/expanders.go @@ -35,8 +35,12 @@ func expandModel(d *schema.ResourceData) *models.TrafficFilterRulesetRequest { } for _, r := range ruleSet.List() { - var m = r.(map[string]interface{}) - var rule = models.TrafficFilterRule{ + m, ok := r.(map[string]interface{}) + if !ok { + continue + } + + rule := models.TrafficFilterRule{ Source: m["source"].(string), } @@ -44,16 +48,16 @@ func expandModel(d *schema.ResourceData) *models.TrafficFilterRulesetRequest { rule.ID = val.(string) } - if val, ok := m["description"]; ok { - rule.Description = val.(string) + if val, ok := m["description"].(string); ok { + rule.Description = val } - if val, ok := m["azure_endpoint_name"]; ok { - rule.AzureEndpointName = val.(string) + if val, ok := m["azure_endpoint_name"].(string); ok { + rule.AzureEndpointName = val } - if val, ok := m["azure_endpoint_guid"]; ok { - rule.AzureEndpointGUID = val.(string) + if val, ok := m["azure_endpoint_guid"].(string); ok { + rule.AzureEndpointGUID = val } request.Rules = append(request.Rules, &rule) diff --git a/ec/ecresource/trafficfilterresource/expanders_test.go b/ec/ecresource/trafficfilterresource/expanders_test.go index 5189ed213..a61d4f1b9 100644 --- a/ec/ecresource/trafficfilterresource/expanders_test.go +++ b/ec/ecresource/trafficfilterresource/expanders_test.go @@ -125,6 +125,36 @@ func Test_expandModel(t *testing.T) { }, }, }, + { + name: "parses an privatelink resource with explicit nils", + args: args{d: util.NewResourceData(t, util.ResDataParams{ + ID: "some-random-id", + State: map[string]interface{}{ + "name": "my traffic filter", + "type": "azure_private_endpoint", + "include_by_default": false, + "region": "azure-australiaeast", + "rule": []interface{}{map[string]interface{}{ + "description": nil, + "azure_endpoint_guid": "1231312-1231-1231-1231-1231312", + "azure_endpoint_name": "my-azure-pl", + }}, + }, + Schema: newSchema(), + })}, + want: &models.TrafficFilterRulesetRequest{ + Name: ec.String("my traffic filter"), + Type: ec.String("azure_private_endpoint"), + IncludeByDefault: ec.Bool(false), + Region: ec.String("azure-australiaeast"), + Rules: []*models.TrafficFilterRule{ + { + AzureEndpointGUID: "1231312-1231-1231-1231-1231312", + AzureEndpointName: "my-azure-pl", + }, + }, + }, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { From f76d47000000a101730ae30af6657c34e2e362ea Mon Sep 17 00:00:00 2001 From: Toby Brain Date: Wed, 14 Sep 2022 20:18:44 +1000 Subject: [PATCH 8/8] Changelog --- .changelog/534.txt | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 .changelog/534.txt diff --git a/.changelog/534.txt b/.changelog/534.txt new file mode 100644 index 000000000..b34035957 --- /dev/null +++ b/.changelog/534.txt @@ -0,0 +1,3 @@ +```release-note:bug +resource: Updates all nested field accesses to validate type casts. This prevents a provider crash when a field is explicitly set to `null`. +```