From 0b89202aff6f897eae92e221882bd7df5eeb0500 Mon Sep 17 00:00:00 2001 From: abarreiro Date: Mon, 20 Nov 2023 12:00:52 +0100 Subject: [PATCH 01/39] Restart PR Signed-off-by: abarreiro --- .changes/v3.11.0/1018-improvements.md | 1 + go.mod | 2 + go.sum | 4 +- vcd/datasource_vcd_rde.go | 1 + vcd/metadata_openapi.go | 293 ++++++++++++++++++++++++++ vcd/metadata_openapi_common_test.go | 230 ++++++++++++++++++++ vcd/resource_vcd_rde.go | 29 +++ vcd/resource_vcd_rde_test.go | 27 +++ website/docs/d/rde.html.markdown | 2 + website/docs/r/rde.html.markdown | 38 ++++ 10 files changed, 625 insertions(+), 2 deletions(-) create mode 100644 .changes/v3.11.0/1018-improvements.md create mode 100644 vcd/metadata_openapi.go create mode 100644 vcd/metadata_openapi_common_test.go diff --git a/.changes/v3.11.0/1018-improvements.md b/.changes/v3.11.0/1018-improvements.md new file mode 100644 index 000000000..9ea44f1e5 --- /dev/null +++ b/.changes/v3.11.0/1018-improvements.md @@ -0,0 +1 @@ +* Add `metadata` attribute to `vcd_rde` to manage metadata entries in Runtime Defined Entities [GH-1018] diff --git a/go.mod b/go.mod index 401d0d4c5..c1cc2e1d0 100644 --- a/go.mod +++ b/go.mod @@ -65,3 +65,5 @@ require ( google.golang.org/grpc v1.57.1 // indirect google.golang.org/protobuf v1.31.0 // indirect ) + +replace github.com/vmware/go-vcloud-director/v2 => github.com/adambarreiro/go-vcloud-director/v2 v2.17.0-alpha.1.0.20231120104249-9e49febd72de diff --git a/go.sum b/go.sum index d304446ec..20618fa53 100644 --- a/go.sum +++ b/go.sum @@ -3,6 +3,8 @@ github.com/Microsoft/go-winio v0.6.1 h1:9/kr64B9VUZrLm5YYwbGtUJnMgqWVOdUAXu6Migc github.com/ProtonMail/go-crypto v0.0.0-20230717121422-5aa5874ade95 h1:KLq8BE0KwCL+mmXnjLWEAOYO+2l2AE4YMmqG1ZpZHBs= github.com/ProtonMail/go-crypto v0.0.0-20230717121422-5aa5874ade95/go.mod h1:EjAoLdwvbIOoOQr3ihjnSoLZRtE8azugULFRteWMNc0= github.com/acomagu/bufpipe v1.0.4 h1:e3H4WUzM3npvo5uv95QuJM3cQspFNtFBzvJ2oNjKIDQ= +github.com/adambarreiro/go-vcloud-director/v2 v2.17.0-alpha.1.0.20231120104249-9e49febd72de h1:pRnEzaUtaEdRpVLsJbDNGIKJ2Hr0DejmtfqoLkSrd7Y= +github.com/adambarreiro/go-vcloud-director/v2 v2.17.0-alpha.1.0.20231120104249-9e49febd72de/go.mod h1:QPxGFgrUcSyzy9IlpwDE4UNT3tsOy2047tJOPEJ4nlw= github.com/agext/levenshtein v1.2.2 h1:0S/Yg6LYmFJ5stwQeRp6EeOcCbj7xiqQSdNelsXvaqE= github.com/agext/levenshtein v1.2.2/go.mod h1:JEDfjyjHDjOF/1e4FlBE/PkbqA9OfWu2ki2W0IB5558= github.com/apparentlymart/go-textseg/v12 v12.0.0/go.mod h1:S/4uRK2UtaQttw1GenVJEynmyUenKwP++x/+DdGV/Ec= @@ -124,8 +126,6 @@ github.com/vmihailenco/msgpack/v5 v5.3.5 h1:5gO0H1iULLWGhs2H5tbAHIZTV8/cYafcFOr9 github.com/vmihailenco/msgpack/v5 v5.3.5/go.mod h1:7xyJ9e+0+9SaZT0Wt1RGleJXzli6Q/V5KbhBonMG9jc= github.com/vmihailenco/tagparser/v2 v2.0.0 h1:y09buUbR+b5aycVFQs/g70pqKVZNBmxwAhO7/IwNM9g= github.com/vmihailenco/tagparser/v2 v2.0.0/go.mod h1:Wri+At7QHww0WTrCBeu4J6bNtoV6mEfg5OIWRZA9qds= -github.com/vmware/go-vcloud-director/v2 v2.22.0-alpha.11 h1:Q71HXQSRSKKf5gcKGHQKvldFRyiOO7MpLJOaIIKCosg= -github.com/vmware/go-vcloud-director/v2 v2.22.0-alpha.11/go.mod h1:QPxGFgrUcSyzy9IlpwDE4UNT3tsOy2047tJOPEJ4nlw= github.com/xanzy/ssh-agent v0.3.3 h1:+/15pJfg/RsTxqYcX6fHqOXZwwMP+2VyYWJeWM2qQFM= github.com/yuin/goldmark v1.4.13/go.mod h1:6yULJ656Px+3vBD8DxQVa3kxgyrAnzto9xy5taEt/CY= github.com/zclconf/go-cty v1.14.0 h1:/Xrd39K7DXbHzlisFP9c4pHao4yyf+/Ug9LEz+Y/yhc= diff --git a/vcd/datasource_vcd_rde.go b/vcd/datasource_vcd_rde.go index bcef88a91..e0f91272c 100644 --- a/vcd/datasource_vcd_rde.go +++ b/vcd/datasource_vcd_rde.go @@ -52,6 +52,7 @@ func datasourceVcdRde() *schema.Resource { Description: "Specifies whether the entity is correctly resolved or not. One of PRE_CREATED, RESOLVED or RESOLUTION_ERROR", Computed: true, }, + "metadata_entry": openApiMetadataEntryDatasourceSchema("Runtime Defined Entity"), }, } } diff --git a/vcd/metadata_openapi.go b/vcd/metadata_openapi.go new file mode 100644 index 000000000..04e4eaa1e --- /dev/null +++ b/vcd/metadata_openapi.go @@ -0,0 +1,293 @@ +package vcd + +import ( + "fmt" + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/validation" + "github.com/vmware/go-vcloud-director/v2/types/v56" + "strconv" +) + +// openApiMetadataEntryDatasourceSchema returns the schema associated to the OpenAPI metadata_entry for a given data source. +// The description will refer to the object name given as input. +func openApiMetadataEntryDatasourceSchema(resourceType string) *schema.Schema { + return &schema.Schema{ + Type: schema.TypeSet, + Computed: true, + Description: fmt.Sprintf("Metadata entries from the given %s", resourceType), + Elem: &schema.Resource{ + Schema: map[string]*schema.Schema{ + "id": { + Type: schema.TypeString, + Computed: true, + Description: "ID of the metadata entry", + }, + "key": { + Type: schema.TypeString, + Computed: true, + Description: "Key of this metadata entry. Required if the metadata entry is not empty", + }, + "value": { + Type: schema.TypeString, + Computed: true, + Description: "Value of this metadata entry. Required if the metadata entry is not empty", + }, + "type": { + Type: schema.TypeString, + Computed: true, + Description: fmt.Sprintf("Type of this metadata entry. One of: '%s', '%s', '%s'", types.OpenApiMetadataStringEntry, types.OpenApiMetadataNumberEntry, types.OpenApiMetadataBooleanEntry), + }, + "readonly": { + Type: schema.TypeBool, + Computed: true, + Description: "True if the metadata entry is read only", + }, + "domain": { + Type: schema.TypeString, + Computed: true, + Description: "Only meaningful for providers. Allows them to share entries with their tenants. One of: `TENANT`, `PROVIDER`", + }, + "namespace": { + Type: schema.TypeString, + Computed: true, + Description: "Namespace of the metadata entry", + }, + }, + }, + } +} + +// openApiMetadataEntryResourceSchema returns the schema associated to the OpenAPI metadata_entry for a given resource. +// The description will refer to the object name given as input. +func openApiMetadataEntryResourceSchema(resourceType string) *schema.Schema { + return &schema.Schema{ + Type: schema.TypeSet, + Optional: true, + Description: fmt.Sprintf("Metadata entries for the given %s", resourceType), + Elem: &schema.Resource{ + Schema: map[string]*schema.Schema{ + "id": { + Type: schema.TypeString, + Computed: true, + Description: "ID of the metadata entry", + }, + "key": { + Type: schema.TypeString, + Required: true, + Description: "Key of this metadata entry. Required if the metadata entry is not empty", + }, + "value": { + Type: schema.TypeString, + Required: true, + Description: "Value of this metadata entry. Required if the metadata entry is not empty", + }, + "type": { + Type: schema.TypeString, + Optional: true, + Default: types.OpenApiMetadataStringEntry, + Description: fmt.Sprintf("Type of this metadata entry. One of: '%s', '%s', '%s'", types.OpenApiMetadataStringEntry, types.OpenApiMetadataNumberEntry, types.OpenApiMetadataBooleanEntry), + ValidateFunc: validation.StringInSlice([]string{types.OpenApiMetadataStringEntry, types.OpenApiMetadataNumberEntry, types.OpenApiMetadataBooleanEntry}, false), + }, + "readonly": { + Type: schema.TypeBool, + Optional: true, + Default: false, + Description: "True if the metadata entry is read only", + }, + "domain": { + Type: schema.TypeString, + Optional: true, + Default: "TENANT", + Description: "Only meaningful for providers. Allows them to share entries with their tenants. Currently, accepted values are: `TENANT`, `PROVIDER`", + ValidateFunc: validation.StringInSlice([]string{"TENANT", "PROVIDER"}, false), + }, + "namespace": { + Type: schema.TypeString, + Optional: true, + Description: "Namespace of the metadata entry", + }, + }, + }, + } +} + +// openApiMetadataCompatible allows to consider all structs that implement OpenAPI metadata handling to be the same type +type openApiMetadataCompatible interface { + GetMetadata() ([]*types.OpenApiMetadataEntry, error) + GetMetadataByKey(key string) (*types.OpenApiMetadataEntry, error) + AddMetadata(metadataEntry types.OpenApiMetadataEntry) (*types.OpenApiMetadataEntry, error) + UpdateMetadata(key string, value interface{}) (*types.OpenApiMetadataEntry, error) + DeleteMetadata(key string) error +} + +// createOrUpdateOpenApiMetadataEntryInVcd creates or updates OpenAPI metadata entries in VCD for the given resource, only if the attribute +// metadata_entry has been set or updated in the state. +func createOrUpdateOpenApiMetadataEntryInVcd(d *schema.ResourceData, resource openApiMetadataCompatible) error { + if !d.HasChange("metadata_entry") { + return nil + } + + oldRaw, newRaw := d.GetChange("metadata_entry") + metadataToAdd, metadataToUpdate, metadataToDelete, err := getMetadataOperations(oldRaw.(*schema.Set).List(), newRaw.(*schema.Set).List()) + if err != nil { + return fmt.Errorf("could not calculate the needed metadata operations: %s", err) + } + + for _, metadataKey := range metadataToDelete { + err := resource.DeleteMetadata(metadataKey) + if err != nil { + return fmt.Errorf("error deleting metadata: %s", err) + } + } + + for metadataKey, metadataEntry := range metadataToUpdate { + _, err := resource.UpdateMetadata(metadataKey, metadataEntry.KeyValue.Value.Value) + if err != nil { + return fmt.Errorf("error updating metadata: %s", err) + } + } + + for _, metadataEntry := range metadataToAdd { + _, err := resource.AddMetadata(metadataEntry) + if err != nil { + return fmt.Errorf("error adding metadata entry: %s", err) + } + } + return nil +} + +// getMetadataOperations retrieves the metadata that needs to be added, to be updated and to be deleted depending +// on the old and new attribute values from Terraform state. +func getMetadataOperations(oldMetadata []interface{}, newMetadata []interface{}) ([]types.OpenApiMetadataEntry, map[string]types.OpenApiMetadataEntry, []string, error) { + oldMetadataEntries, err := getOpenApiMetadataEntryMap(oldMetadata) + if err != nil { + return nil, nil, nil, err + } + newMetadataEntries, err := getOpenApiMetadataEntryMap(newMetadata) + if err != nil { + return nil, nil, nil, err + } + + var metadataToRemove []string + for oldKey := range oldMetadataEntries { + if _, ok := newMetadataEntries[oldKey]; !ok { + metadataToRemove = append(metadataToRemove, oldKey) + } + } + + metadataToUpdate := map[string]types.OpenApiMetadataEntry{} + for newKey, newEntry := range newMetadataEntries { + if oldEntry, ok := oldMetadataEntries[newKey]; ok { + if oldEntry.KeyValue.Value == newEntry.KeyValue.Value.Value { + continue + } + metadataToUpdate[newKey] = newEntry + } + } + + var metadataToCreate []types.OpenApiMetadataEntry + for newKey, newEntry := range newMetadataEntries { + if _, ok := metadataToUpdate[newKey]; !ok { + metadataToCreate = append(metadataToCreate, newEntry) + } + } + + return metadataToCreate, metadataToUpdate, metadataToRemove, nil +} + +// getOpenApiMetadataKeySet converts the input metadata attribute from Terraform state to a map composed by metadata keys and their values. +func getOpenApiMetadataEntryMap(metadataAttribute []interface{}) (map[string]types.OpenApiMetadataEntry, error) { + metadataMap := map[string]types.OpenApiMetadataEntry{} + for _, rawItem := range metadataAttribute { + metadataEntry := rawItem.(map[string]interface{}) + + namespace := "" + if _, ok := metadataEntry["namespace"]; ok { + namespace = metadataEntry["namespace"].(string) + } + + value, err := convertOpenApiMetadataValue(metadataEntry["type"].(string), metadataEntry["value"].(string)) + if err != nil { + return nil, fmt.Errorf("error parsing the 'value' attribute '%s' from state: %s", metadataEntry["value"].(string), err) + } + + metadataMap[metadataEntry["key"].(string)] = types.OpenApiMetadataEntry{ + IsReadOnly: metadataEntry["readonly"].(bool), // Should be always populated as it has a default value + KeyValue: types.OpenApiMetadataKeyValue{ + Domain: metadataEntry["domain"].(string), // Should be always populated as it has a default value + Key: metadataEntry["key"].(string), // Should be always populated as it is required + Value: types.OpenApiMetadataTypedValue{ + Value: value, + Type: metadataEntry["type"].(string), // Should be always populated as it has a default value + }, + Namespace: namespace, + }, + } + } + return metadataMap, nil +} + +// updateOpenApiMetadataInState updates metadata and metadata_entry in the Terraform state for the given receiver object. +// This can be done as both are Computed, for compatibility reasons. +func updateOpenApiMetadataInState(d *schema.ResourceData, receiverObject openApiMetadataCompatible) error { + allMetadata, err := receiverObject.GetMetadata() + if err != nil { + return err + } + + if len(allMetadata) == 0 { + return nil + } + + metadata := make([]interface{}, len(allMetadata)) + for i, metadataEntryFromVcd := range allMetadata { + // We need to set the correct type, otherwise saving the state will fail + value := "" + switch metadataEntryFromVcd.KeyValue.Value.Type { + case types.OpenApiMetadataBooleanEntry: + value = fmt.Sprintf("%t", metadataEntryFromVcd.KeyValue.Value.Value.(bool)) + case types.OpenApiMetadataNumberEntry: + // OpenAPI doesn't + value = fmt.Sprintf("%.0f", metadataEntryFromVcd.KeyValue.Value.Value.(float64)) + case types.OpenApiMetadataStringEntry: + value = metadataEntryFromVcd.KeyValue.Value.Value.(string) + default: + return fmt.Errorf("not supported metadata type %s", metadataEntryFromVcd.KeyValue.Value.Type) + } + + metadataEntry := map[string]interface{}{ + "id": metadataEntryFromVcd.ID, + "key": metadataEntryFromVcd.KeyValue.Key, + "readonly": metadataEntryFromVcd.IsReadOnly, + "domain": metadataEntryFromVcd.KeyValue.Domain, + "namespace": metadataEntryFromVcd.KeyValue.Namespace, + "type": metadataEntryFromVcd.KeyValue.Value.Type, + "value": value, + } + metadata[i] = metadataEntry + } + + err = d.Set("metadata_entry", metadata) + return err +} + +// convertOpenApiMetadataValue converts a metadata value from plain string to a correct typed value that can be sent +// in OpenAPI payloads. +func convertOpenApiMetadataValue(valueType, value string) (interface{}, error) { + var convertedValue interface{} + var err error + switch valueType { + case types.OpenApiMetadataStringEntry: + convertedValue = value + case types.OpenApiMetadataNumberEntry: + convertedValue, err = strconv.ParseFloat(value, 64) + case types.OpenApiMetadataBooleanEntry: + convertedValue, err = strconv.ParseBool(value) + default: + return nil, fmt.Errorf("unrecognized metadata type %s", valueType) + } + if err != nil { + return nil, fmt.Errorf("error parsing the value '%v': %s", value, err) + } + return convertedValue, nil +} diff --git a/vcd/metadata_openapi_common_test.go b/vcd/metadata_openapi_common_test.go new file mode 100644 index 000000000..c52f8816c --- /dev/null +++ b/vcd/metadata_openapi_common_test.go @@ -0,0 +1,230 @@ +//go:build rde || functional || ALL + +package vcd + +import ( + "fmt" + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" + "github.com/vmware/go-vcloud-director/v2/types/v56" + "strconv" + "strings" + "testing" +) + +// testOpenApiMetadataEntryCRUD executes a test that asserts CRUD operation behaviours of "metadata_entry" attribute in the given HCL +// templates, that must correspond to a resource and a data source referencing this resource. +// The HCL template requires {{.Name}} and {{.Metadata}} fields, and the usual {{.Org}} and {{.Vdc}}. +// You can add extra parameters as well to inject in the given HCL template, or override these mentioned ones. +// The data source HCL is always concatenated to the resource after creation, and it's skipped on binary tests. +// +// Tests: +// - Step 1: Create the resource with no metadata +// - Step 2: Taint and re-create with 4 metadata entries, 1 for string, number, bool, date with GENERAL domain (is_system = false) +// - Step 3: Add a data source +// - Step 4: Delete 1 metadata entry, the bool one +// - Step 5: Update the string and date metadata values +// - Step 6: Delete all of them +// - Step 7: (Sysadmin only) Create 2 entries with is_system=true (readonly and private user_access) +// - Step 8: (Sysadmin only) Update the hidden one +// - Step 9: (Sysadmin only) Delete all of them +// - Step 10: Check a malformed metadata entry +// - Step 11: (Org user only) Check that specifying an is_system metadata entry with a tenant user gives an error +// - Step 12+: Some extra tests for deprecated `metadata` attribute +func testOpenApiMetadataEntryCRUD(t *testing.T, resourceTemplate, resourceAddress, datasourceTemplate, datasourceAddress string, extraParams StringMap) { + preTestChecks(t) + var params = StringMap{ + "Org": testConfig.VCD.Org, + "Vdc": testConfig.Nsxt.Vdc, + "Name": t.Name(), + } + + for extraParam, extraParamValue := range extraParams { + params[extraParam] = extraParamValue + } + testParamsNotEmpty(t, params) + + params["FuncName"] = t.Name() + "NoMetadata" + params["Metadata"] = " " + noMetadataHcl := templateFill(resourceTemplate, params) + debugPrintf("#[DEBUG] CONFIGURATION: %s", noMetadataHcl) + + params["FuncName"] = t.Name() + "Create" + params["Metadata"] = getOpenApiMetadataTestingHcl(1, 1, 1, 1, 1, 1) + createHcl := templateFill(resourceTemplate, params) + debugPrintf("#[DEBUG] CONFIGURATION: %s", createHcl) + + withDatasourceHcl := "" + if datasourceTemplate != "" { + params["FuncName"] = t.Name() + "WithDatasource" + withDatasourceHcl = templateFill(datasourceTemplate+"\n# skip-binary-test\n"+resourceTemplate, params) + debugPrintf("#[DEBUG] CONFIGURATION: %s", withDatasourceHcl) + } + + params["FuncName"] = t.Name() + "DeleteOneKey" + params["Metadata"] = getOpenApiMetadataTestingHcl(1, 1, 0, 1, 1, 1) + deleteOneKeyHcl := templateFill(resourceTemplate, params) + debugPrintf("#[DEBUG] CONFIGURATION: %s", deleteOneKeyHcl) + + params["FuncName"] = t.Name() + "Update" + params["Metadata"] = strings.NewReplacer("stringValue", "stringValueUpdated").Replace(params["Metadata"].(string)) + updateHcl := templateFill(resourceTemplate, params) + debugPrintf("#[DEBUG] CONFIGURATION: %s", updateHcl) + + params["FuncName"] = t.Name() + "Delete" + params["Metadata"] = " " + deleteHcl := templateFill(resourceTemplate, params) + debugPrintf("#[DEBUG] CONFIGURATION: %s", deleteHcl) + + params["FuncName"] = t.Name() + "MetadataEntryWithDefaults" + params["Metadata"] = "metadata_entry {\n\tkey = \"defaultKey\"\nvalue = \"defaultValue\"\n}" + withDefaults := templateFill(resourceTemplate, params) + debugPrintf("#[DEBUG] CONFIGURATION: %s", withDefaults) + + if vcdShortTest { + t.Skip(acceptanceTestsSkipped) + return + } + + resource.Test(t, resource.TestCase{ + ProviderFactories: testAccProviders, + Steps: []resource.TestStep{ + { + Config: noMetadataHcl, + Check: resource.ComposeAggregateTestCheckFunc( + resource.TestCheckResourceAttr(resourceAddress, "name", t.Name()), + resource.TestCheckResourceAttr(resourceAddress, "metadata_entry.#", "0"), + ), + }, + { + Config: createHcl, + Taint: []string{resourceAddress}, // Forces re-creation to test Create with metadata. + Check: resource.ComposeAggregateTestCheckFunc( + resource.TestCheckResourceAttr(resourceAddress, "name", t.Name()), + resource.TestCheckResourceAttr(resourceAddress, "metadata_entry.#", "6"), + testCheckOpenApiMetadataEntrySetElemNestedAttrs(resourceAddress, "stringKey1", "stringValue1", types.OpenApiMetadataStringEntry, "TENANT", "", "false"), + testCheckOpenApiMetadataEntrySetElemNestedAttrs(resourceAddress, "numberKey1", "1", types.OpenApiMetadataNumberEntry, "TENANT", "", "false"), + testCheckOpenApiMetadataEntrySetElemNestedAttrs(resourceAddress, "boolKey1", "false", types.OpenApiMetadataStringEntry, "TENANT", "", "false"), + testCheckOpenApiMetadataEntrySetElemNestedAttrs(resourceAddress, "readOnly1", "readOnly1", types.OpenApiMetadataStringEntry, "TENANT", "", "true"), + testCheckOpenApiMetadataEntrySetElemNestedAttrs(resourceAddress, "namespaced1", "namespaced1", types.OpenApiMetadataStringEntry, "TENANT", "namespace", "false"), + testCheckOpenApiMetadataEntrySetElemNestedAttrs(resourceAddress, "provider1", "provider1", types.OpenApiMetadataStringEntry, "PROVIDER", "", "false"), + ), + }, + { + SkipFunc: func() (bool, error) { + return withDatasourceHcl == "", nil + }, + Config: withDatasourceHcl, + Check: resource.ComposeAggregateTestCheckFunc( + resource.TestCheckResourceAttrPair(datasourceAddress, "id", resourceAddress, "id"), + resource.TestCheckResourceAttr(resourceAddress, "metadata_entry.#", "6"), + resource.TestCheckResourceAttr(datasourceAddress, "metadata_entry.#", "6"), + testCheckOpenApiMetadataEntrySetElemNestedAttrs(resourceAddress, "stringKey1", "stringValue1", types.OpenApiMetadataStringEntry, "TENANT", "", "false"), + testCheckOpenApiMetadataEntrySetElemNestedAttrs(resourceAddress, "numberKey1", "1", types.OpenApiMetadataNumberEntry, "TENANT", "", "false"), + testCheckOpenApiMetadataEntrySetElemNestedAttrs(resourceAddress, "boolKey1", "false", types.OpenApiMetadataBooleanEntry, "TENANT", "", "false"), + testCheckOpenApiMetadataEntrySetElemNestedAttrs(resourceAddress, "readOnly1", "readOnly1", types.OpenApiMetadataStringEntry, "TENANT", "", "true"), + testCheckOpenApiMetadataEntrySetElemNestedAttrs(resourceAddress, "namespaced1", "namespaced1", types.OpenApiMetadataStringEntry, "TENANT", "namespace", "false"), + testCheckOpenApiMetadataEntrySetElemNestedAttrs(resourceAddress, "provider1", "provider1", types.OpenApiMetadataStringEntry, "PROVIDER", "", "false"), + ), + }, + { + Config: deleteOneKeyHcl, + Check: resource.ComposeAggregateTestCheckFunc( + resource.TestCheckResourceAttr(resourceAddress, "name", t.Name()), + resource.TestCheckResourceAttr(resourceAddress, "metadata_entry.#", "5"), + // The bool is deleted + testCheckOpenApiMetadataEntrySetElemNestedAttrs(resourceAddress, "stringKey1", "stringValue1", types.OpenApiMetadataStringEntry, "TENANT", "", "false"), + testCheckOpenApiMetadataEntrySetElemNestedAttrs(resourceAddress, "numberKey1", "1", types.OpenApiMetadataNumberEntry, "TENANT", "", "false"), + testCheckOpenApiMetadataEntrySetElemNestedAttrs(resourceAddress, "readOnly1", "readOnly1", types.OpenApiMetadataStringEntry, "TENANT", "", "true"), + testCheckOpenApiMetadataEntrySetElemNestedAttrs(resourceAddress, "namespaced1", "namespaced1", types.OpenApiMetadataStringEntry, "TENANT", "namespace", "false"), + testCheckOpenApiMetadataEntrySetElemNestedAttrs(resourceAddress, "provider1", "provider1", types.OpenApiMetadataStringEntry, "PROVIDER", "", "false"), + ), + }, + { + Config: updateHcl, + Check: resource.ComposeAggregateTestCheckFunc( + resource.TestCheckResourceAttr(resourceAddress, "name", t.Name()), + resource.TestCheckResourceAttr(resourceAddress, "metadata_entry.#", "5"), + // Updated value: + testCheckOpenApiMetadataEntrySetElemNestedAttrs(resourceAddress, "stringKey1", "stringValueUpdated1", types.OpenApiMetadataStringEntry, "TENANT", "", "false"), + // Not updated values: + testCheckOpenApiMetadataEntrySetElemNestedAttrs(resourceAddress, "numberKey1", "1", types.OpenApiMetadataNumberEntry, "TENANT", "", "false"), + testCheckOpenApiMetadataEntrySetElemNestedAttrs(resourceAddress, "readOnly1", "readOnly1", types.OpenApiMetadataStringEntry, "TENANT", "", "true"), + testCheckOpenApiMetadataEntrySetElemNestedAttrs(resourceAddress, "namespaced1", "namespaced1", types.OpenApiMetadataStringEntry, "TENANT", "namespace", "false"), + testCheckOpenApiMetadataEntrySetElemNestedAttrs(resourceAddress, "provider1", "provider1", types.OpenApiMetadataStringEntry, "PROVIDER", "", "false"), + ), + }, + { + Config: deleteHcl, + Check: resource.ComposeAggregateTestCheckFunc( + resource.TestCheckResourceAttr(resourceAddress, "name", t.Name()), + resource.TestCheckResourceAttr(resourceAddress, "metadata_entry.#", "0"), + ), + }, + { + Config: withDefaults, + Check: resource.ComposeAggregateTestCheckFunc( + resource.TestCheckResourceAttr(resourceAddress, "name", t.Name()), + resource.TestCheckResourceAttr(resourceAddress, "metadata_entry.#", "1"), + testCheckOpenApiMetadataEntrySetElemNestedAttrs(resourceAddress, "defaultKey", "defaultValue", types.OpenApiMetadataStringEntry, "TENANT", "", "false"), + ), + }, + }, + }) + postTestChecks(t) +} + +// getOpenApiMetadataTestingHcl gets valid metadata entries to inject them into an HCL for testing +func getOpenApiMetadataTestingHcl(stringEntries, numberEntries, boolEntries, readonlyEntries, namespacedEntries, providerEntries int) string { + hcl := "" + for i := 1; i <= stringEntries; i++ { + hcl += getOpenApiMetadataEntryHcl(fmt.Sprintf("stringKey%d", i), fmt.Sprintf("stringValue%d", i), types.OpenApiMetadataStringEntry, "TENANT", "", "false") + } + for i := 1; i <= numberEntries; i++ { + hcl += getOpenApiMetadataEntryHcl(fmt.Sprintf("numberKey%d", i), fmt.Sprintf("%d", i), types.OpenApiMetadataNumberEntry, "TENANT", "", "false") + } + for i := 1; i <= boolEntries; i++ { + hcl += getOpenApiMetadataEntryHcl(fmt.Sprintf("boolKey%d", i), strconv.FormatBool(i%2 == 0), types.OpenApiMetadataBooleanEntry, "TENANT", "", "false") + } + for i := 1; i <= readonlyEntries; i++ { + hcl += getOpenApiMetadataEntryHcl(fmt.Sprintf("readOnly%d", i), fmt.Sprintf("readOnly%d", i), types.OpenApiMetadataStringEntry, "TENANT", "", "true") + } + for i := 1; i <= namespacedEntries; i++ { + hcl += getOpenApiMetadataEntryHcl(fmt.Sprintf("namespaced%d", i), fmt.Sprintf("namespaced%d", i), types.OpenApiMetadataStringEntry, "TENANT", "namespace", "false") + } + for i := 1; i <= providerEntries; i++ { + hcl += getOpenApiMetadataEntryHcl(fmt.Sprintf("provider%d", i), fmt.Sprintf("provider%d", i), types.OpenApiMetadataStringEntry, "PROVIDER", "", "false") + } + return hcl + +} + +func getOpenApiMetadataEntryHcl(key, value, typedValue, domain, namespace, readonly string) string { + hclNamespace := "" + if namespace != "" { + hclNamespace = `namespace = "` + namespace + `"` + } + + return ` + metadata_entry { + key = "` + key + `" + value = "` + value + `" + type = "` + typedValue + `" + domain = "` + domain + `" + ` + hclNamespace + ` + readonly = ` + readonly + ` + }` +} + +// testCheckOpenApiMetadataEntrySetElemNestedAttrs asserts that a given metadata_entry has the expected input for the given resourceAddress. +func testCheckOpenApiMetadataEntrySetElemNestedAttrs(resourceAddress, expectedKey, expectedValue, expectedType, expectedDomain, expectedNamespace, expectedReadonly string) resource.TestCheckFunc { + return resource.TestCheckTypeSetElemNestedAttrs(resourceAddress, "metadata_entry.*", + map[string]string{ + "key": expectedKey, + "value": expectedValue, + "type": expectedType, + "domain": expectedDomain, + "readonly": expectedReadonly, + "namespace": expectedNamespace, + }, + ) +} diff --git a/vcd/resource_vcd_rde.go b/vcd/resource_vcd_rde.go index 5c5db2aa3..6cc4e165c 100644 --- a/vcd/resource_vcd_rde.go +++ b/vcd/resource_vcd_rde.go @@ -102,6 +102,7 @@ func resourceVcdRde() *schema.Resource { Description: "If true, `computed_entity` is equal to either `input_entity` or the contents of `input_entity_url`", Computed: true, }, + "metadata_entry": openApiMetadataEntryResourceSchema("Runtime Defined Entity"), }, } } @@ -149,6 +150,16 @@ func resourceVcdRdeCreate(ctx context.Context, d *schema.ResourceData, meta inte } } + // Metadata is only supported since v37.0 + if vcdClient.Client.APIVCDMaxVersionIs(">= 37.0") { + err = createOrUpdateOpenApiMetadataEntryInVcd(d, rde) + if err != nil { + return diag.Errorf("could not create metadata for the Runtime Defined Entity: %s", err) + } + } else if _, ok := d.GetOk("metadata_entry"); ok { + return diag.Errorf("metadata_entry is only supported since VCD 10.4.0") + } + return resourceVcdRdeRead(ctx, d, meta) } @@ -228,6 +239,14 @@ func resourceVcdRdeRead(_ context.Context, d *schema.ResourceData, meta interfac dSet(d, "entity_in_sync", areJsonEqual) } + // Metadata is only available since API v37.0 + if vcdClient.Client.APIVCDMaxVersionIs(">= 37.0") { + err = updateOpenApiMetadataInState(d, rde) + if err != nil { + return diag.Errorf("could not set metadata for the Runtime Defined Entity: %s", err) + } + } + d.SetId(rde.DefinedEntity.ID) return nil @@ -320,6 +339,16 @@ func resourceVcdRdeUpdate(ctx context.Context, d *schema.ResourceData, meta inte } } + // Metadata is only supported since v37.0 + if vcdClient.Client.APIVCDMaxVersionIs(">= 37.0") { + err = createOrUpdateOpenApiMetadataEntryInVcd(d, rde) + if err != nil { + return diag.Errorf("could not create metadata for the Runtime Defined Entity: %s", err) + } + } else if _, ok := d.GetOk("metadata_entry"); ok { + return diag.Errorf("metadata_entry is only supported since VCD 10.4.0") + } + return resourceVcdRdeRead(ctx, d, meta) } diff --git a/vcd/resource_vcd_rde_test.go b/vcd/resource_vcd_rde_test.go index 44fbdca12..9bb9f15f1 100644 --- a/vcd/resource_vcd_rde_test.go +++ b/vcd/resource_vcd_rde_test.go @@ -536,3 +536,30 @@ func manipulateRde(t *testing.T, vcdClient *VCDClient, rdeId string) { t.Fatalf("could not update RDE with ID '%s': %s", rdeId, err) } } + +// TestAccVcdRdeMetadata tests metadata CRUD on Runtime Defined Entities. +func TestAccVcdRdeMetadata(t *testing.T) { + skipIfNotSysAdmin(t) + vcdClient := createTemporaryVCDConnection(true) + if vcdClient != nil && vcdClient.Client.APIVCDMaxVersionIs("< 37.0") { + t.Skip("skipped as metadata for vcd_rde is only supported since VCD 10.4.0") + } + testOpenApiMetadataEntryCRUD(t, + testAccCheckVcdRdeMetadata, "vcd_rde.test-rde", + "", "", + StringMap{}) +} + +const testAccCheckVcdRdeMetadata = ` +data "vcd_rde_type" "rde_type" { + vendor = "vmware" + nss = "tkgcluster" + version = "1.0.0" +} +resource "vcd_rde" "test-rde" { + rde_type_id = data.vcd_rde_type.rde_type.id + name = "{{.Name}}" + input_entity = "{\"foo\":\"bar\"}" # We are just testing metadata so we don't care about entity state + {{.Metadata}} +} +` diff --git a/website/docs/d/rde.html.markdown b/website/docs/d/rde.html.markdown index 49206b859..a0508fb52 100644 --- a/website/docs/d/rde.html.markdown +++ b/website/docs/d/rde.html.markdown @@ -52,3 +52,5 @@ The following attributes are supported: * `owner_user_id` - The ID of the [Organization user](/providers/vmware/vcd/latest/docs/resources/org_user) that owns this Runtime Defined Entity. * `org_id` - The ID of the [Organization](/providers/vmware/vcd/latest/docs/resources/org) to which the Runtime Defined Entity belongs. * `state` - It can be `RESOLVED`, `RESOLUTION_ERROR` or `PRE_CREATED`. +* `metadata_entry` - A set of metadata entries that belong to the RDE. + Read the [resource](/providers/vmware/vcd/latest/docs/resources/rde#metadata) documentation for the details of the sub-attributes. diff --git a/website/docs/r/rde.html.markdown b/website/docs/r/rde.html.markdown index 85632955b..3f883d8ef 100644 --- a/website/docs/r/rde.html.markdown +++ b/website/docs/r/rde.html.markdown @@ -131,6 +131,7 @@ The following arguments are supported: The referenced JSON will be downloaded on every read operation, and it will break Terraform operations if these contents are no longer present on the remote site. If you can't guarantee this, it is safer to use `input_entity`. * `external_id` - (Optional) An external input_entity's ID that this Runtime Defined Entity may have a relation to. +* `metadata_entry` - (Optional; *v3.11+*) A set of metadata entries to assign. See [Metadata](#metadata) section for details. ## Attribute Reference @@ -185,6 +186,43 @@ should have `resolve=false` to avoid being resolved). In this last scenario, it is advisable to mark `resolve_on_removal=true` so Terraform can delete the RDE even if it was not resolved by anyone. + +## Metadata + +The `metadata_entry` is a set of metadata entries that have the following structure: + +* `key` - (Required) Key of this metadata entry. +* `value` - (Required) Value of this metadata entry. +* `type` - (Optional) Type of this metadata entry. One of: `StringEntry`, `NumberEntry`, `BoolEntry`. Defaults to `StringEntry`. +* `domain` - (Optional) Only meaningful for providers. Allows them to share entries with their tenants. Currently, accepted values are: `TENANT`, `PROVIDER`. Defaults to `TENANT`. +* `readonly` - (Optional) `true` if the metadata entry is read only. Defaults to `false`. +* `id` - (Computed) Read-only identifier for this metadata entry. + +Example: + +```hcl +resource "vcd_rde" "my-rde" { + org = "my-org" + rde_type_id = data.vcd_rde_type.my-type.id + name = "My custom RDE" + resolve = true + entity_url = "https://just.an-example.com/entities/custom-rde.json" + metadata_entry { + key = "foo" + type = "StringEntry" + value = "bar" + domain = "TENANT" + readonly = true + } + metadata_entry { + key = "bar" + type = "NumberEntry" + value = "42" + domain = "TENANT" + readonly = true + } +} +``` ## Importing From 219e80b62443dcc066a0ec64c40aec3aa5b9ace2 Mon Sep 17 00:00:00 2001 From: abarreiro Date: Mon, 20 Nov 2023 12:17:43 +0100 Subject: [PATCH 02/39] Fix tests Signed-off-by: abarreiro --- vcd/datasource_vcd_rde.go | 8 ++++++++ vcd/metadata_openapi_common_test.go | 12 +++--------- vcd/resource_vcd_rde_test.go | 12 +++++++++++- 3 files changed, 22 insertions(+), 10 deletions(-) diff --git a/vcd/datasource_vcd_rde.go b/vcd/datasource_vcd_rde.go index e0f91272c..ad47230a6 100644 --- a/vcd/datasource_vcd_rde.go +++ b/vcd/datasource_vcd_rde.go @@ -84,6 +84,14 @@ func datasourceVcdRdeRead(_ context.Context, d *schema.ResourceData, meta interf dSet(d, "owner_user_id", rde.DefinedEntity.Owner.ID) } + // Metadata is only available since API v37.0 + if vcdClient.Client.APIVCDMaxVersionIs(">= 37.0") { + err = updateOpenApiMetadataInState(d, rde) + if err != nil { + return diag.Errorf("could not set metadata for the Runtime Defined Entity: %s", err) + } + } + d.SetId(rde.DefinedEntity.ID) return nil diff --git a/vcd/metadata_openapi_common_test.go b/vcd/metadata_openapi_common_test.go index c52f8816c..9ef4ac40d 100644 --- a/vcd/metadata_openapi_common_test.go +++ b/vcd/metadata_openapi_common_test.go @@ -53,12 +53,9 @@ func testOpenApiMetadataEntryCRUD(t *testing.T, resourceTemplate, resourceAddres createHcl := templateFill(resourceTemplate, params) debugPrintf("#[DEBUG] CONFIGURATION: %s", createHcl) - withDatasourceHcl := "" - if datasourceTemplate != "" { - params["FuncName"] = t.Name() + "WithDatasource" - withDatasourceHcl = templateFill(datasourceTemplate+"\n# skip-binary-test\n"+resourceTemplate, params) - debugPrintf("#[DEBUG] CONFIGURATION: %s", withDatasourceHcl) - } + params["FuncName"] = t.Name() + "WithDatasource" + withDatasourceHcl := templateFill(datasourceTemplate+"\n# skip-binary-test\n"+resourceTemplate, params) + debugPrintf("#[DEBUG] CONFIGURATION: %s", withDatasourceHcl) params["FuncName"] = t.Name() + "DeleteOneKey" params["Metadata"] = getOpenApiMetadataTestingHcl(1, 1, 0, 1, 1, 1) @@ -110,9 +107,6 @@ func testOpenApiMetadataEntryCRUD(t *testing.T, resourceTemplate, resourceAddres ), }, { - SkipFunc: func() (bool, error) { - return withDatasourceHcl == "", nil - }, Config: withDatasourceHcl, Check: resource.ComposeAggregateTestCheckFunc( resource.TestCheckResourceAttrPair(datasourceAddress, "id", resourceAddress, "id"), diff --git a/vcd/resource_vcd_rde_test.go b/vcd/resource_vcd_rde_test.go index 9bb9f15f1..8f83a2b25 100644 --- a/vcd/resource_vcd_rde_test.go +++ b/vcd/resource_vcd_rde_test.go @@ -546,7 +546,7 @@ func TestAccVcdRdeMetadata(t *testing.T) { } testOpenApiMetadataEntryCRUD(t, testAccCheckVcdRdeMetadata, "vcd_rde.test-rde", - "", "", + testAccCheckVcdRdeMetadataDatasource, "data.vcd_rde.test-rde-ds", StringMap{}) } @@ -557,9 +557,19 @@ data "vcd_rde_type" "rde_type" { version = "1.0.0" } resource "vcd_rde" "test-rde" { + org = "System" rde_type_id = data.vcd_rde_type.rde_type.id name = "{{.Name}}" input_entity = "{\"foo\":\"bar\"}" # We are just testing metadata so we don't care about entity state + resolve = true {{.Metadata}} } ` + +const testAccCheckVcdRdeMetadataDatasource = ` +data "vcd_rde" "test-rde-ds" { + org = "System" + rde_type_id = vcd_rde.test-rde.rde_type_id + name = vcd_rde.test-rde.name +} +` From 9da22b6aeb46b045522250695373fd8d40d6c2d6 Mon Sep 17 00:00:00 2001 From: abarreiro Date: Mon, 20 Nov 2023 12:22:22 +0100 Subject: [PATCH 03/39] Self review Signed-off-by: abarreiro --- .changes/v3.11.0/1018-improvements.md | 2 +- vcd/metadata_openapi.go | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/.changes/v3.11.0/1018-improvements.md b/.changes/v3.11.0/1018-improvements.md index 9ea44f1e5..b2276fc7f 100644 --- a/.changes/v3.11.0/1018-improvements.md +++ b/.changes/v3.11.0/1018-improvements.md @@ -1 +1 @@ -* Add `metadata` attribute to `vcd_rde` to manage metadata entries in Runtime Defined Entities [GH-1018] +* Add `metadata_entry` attribute to `vcd_rde` resource and data source to manage metadata entries in Runtime Defined Entities [GH-1018] diff --git a/vcd/metadata_openapi.go b/vcd/metadata_openapi.go index 04e4eaa1e..5596f48e6 100644 --- a/vcd/metadata_openapi.go +++ b/vcd/metadata_openapi.go @@ -9,7 +9,7 @@ import ( ) // openApiMetadataEntryDatasourceSchema returns the schema associated to the OpenAPI metadata_entry for a given data source. -// The description will refer to the object name given as input. +// The description will refer to the object type given as input. func openApiMetadataEntryDatasourceSchema(resourceType string) *schema.Schema { return &schema.Schema{ Type: schema.TypeSet, @@ -25,12 +25,12 @@ func openApiMetadataEntryDatasourceSchema(resourceType string) *schema.Schema { "key": { Type: schema.TypeString, Computed: true, - Description: "Key of this metadata entry. Required if the metadata entry is not empty", + Description: "Key of this metadata entry", }, "value": { Type: schema.TypeString, Computed: true, - Description: "Value of this metadata entry. Required if the metadata entry is not empty", + Description: "Value of this metadata entry", }, "type": { Type: schema.TypeString, From db3d9261c68d11d8e174dc8bf2c6b0c3eb437bfd Mon Sep 17 00:00:00 2001 From: abarreiro Date: Mon, 20 Nov 2023 12:25:16 +0100 Subject: [PATCH 04/39] Self review Signed-off-by: abarreiro --- vcd/metadata_openapi_common_test.go | 14 -------------- 1 file changed, 14 deletions(-) diff --git a/vcd/metadata_openapi_common_test.go b/vcd/metadata_openapi_common_test.go index 9ef4ac40d..f12a3b280 100644 --- a/vcd/metadata_openapi_common_test.go +++ b/vcd/metadata_openapi_common_test.go @@ -16,20 +16,6 @@ import ( // The HCL template requires {{.Name}} and {{.Metadata}} fields, and the usual {{.Org}} and {{.Vdc}}. // You can add extra parameters as well to inject in the given HCL template, or override these mentioned ones. // The data source HCL is always concatenated to the resource after creation, and it's skipped on binary tests. -// -// Tests: -// - Step 1: Create the resource with no metadata -// - Step 2: Taint and re-create with 4 metadata entries, 1 for string, number, bool, date with GENERAL domain (is_system = false) -// - Step 3: Add a data source -// - Step 4: Delete 1 metadata entry, the bool one -// - Step 5: Update the string and date metadata values -// - Step 6: Delete all of them -// - Step 7: (Sysadmin only) Create 2 entries with is_system=true (readonly and private user_access) -// - Step 8: (Sysadmin only) Update the hidden one -// - Step 9: (Sysadmin only) Delete all of them -// - Step 10: Check a malformed metadata entry -// - Step 11: (Org user only) Check that specifying an is_system metadata entry with a tenant user gives an error -// - Step 12+: Some extra tests for deprecated `metadata` attribute func testOpenApiMetadataEntryCRUD(t *testing.T, resourceTemplate, resourceAddress, datasourceTemplate, datasourceAddress string, extraParams StringMap) { preTestChecks(t) var params = StringMap{ From 2c20fd9869a95df9c5d00dc37361ae50481f92fa Mon Sep 17 00:00:00 2001 From: abarreiro Date: Tue, 21 Nov 2023 12:38:01 +0100 Subject: [PATCH 05/39] Add persistent attr Signed-off-by: abarreiro --- vcd/metadata_openapi.go | 34 ++++++---- vcd/metadata_openapi_common_test.go | 99 ++++++++++++++++------------- website/docs/r/rde.html.markdown | 2 + 3 files changed, 79 insertions(+), 56 deletions(-) diff --git a/vcd/metadata_openapi.go b/vcd/metadata_openapi.go index 5596f48e6..d3655e452 100644 --- a/vcd/metadata_openapi.go +++ b/vcd/metadata_openapi.go @@ -52,6 +52,11 @@ func openApiMetadataEntryDatasourceSchema(resourceType string) *schema.Schema { Computed: true, Description: "Namespace of the metadata entry", }, + "persistent": { + Type: schema.TypeBool, + Computed: true, + Description: "Persistent metadata entries can be copied over on some entity operation", + }, }, }, } @@ -106,6 +111,11 @@ func openApiMetadataEntryResourceSchema(resourceType string) *schema.Schema { Optional: true, Description: "Namespace of the metadata entry", }, + "persistent": { + Type: schema.TypeBool, + Optional: true, + Description: "Persistent metadata entries can be copied over on some entity operation", + }, }, }, } @@ -212,13 +222,14 @@ func getOpenApiMetadataEntryMap(metadataAttribute []interface{}) (map[string]typ } metadataMap[metadataEntry["key"].(string)] = types.OpenApiMetadataEntry{ - IsReadOnly: metadataEntry["readonly"].(bool), // Should be always populated as it has a default value + IsReadOnly: metadataEntry["readonly"].(bool), // It is always populated as it has a default value + IsPersistent: metadataEntry["persistent"].(bool), // It is always populated as it has a default value KeyValue: types.OpenApiMetadataKeyValue{ - Domain: metadataEntry["domain"].(string), // Should be always populated as it has a default value - Key: metadataEntry["key"].(string), // Should be always populated as it is required + Domain: metadataEntry["domain"].(string), // It is always populated as it has a default value + Key: metadataEntry["key"].(string), // It is always populated as it is required Value: types.OpenApiMetadataTypedValue{ Value: value, - Type: metadataEntry["type"].(string), // Should be always populated as it has a default value + Type: metadataEntry["type"].(string), // It is always populated as it has a default value }, Namespace: namespace, }, @@ -256,13 +267,14 @@ func updateOpenApiMetadataInState(d *schema.ResourceData, receiverObject openApi } metadataEntry := map[string]interface{}{ - "id": metadataEntryFromVcd.ID, - "key": metadataEntryFromVcd.KeyValue.Key, - "readonly": metadataEntryFromVcd.IsReadOnly, - "domain": metadataEntryFromVcd.KeyValue.Domain, - "namespace": metadataEntryFromVcd.KeyValue.Namespace, - "type": metadataEntryFromVcd.KeyValue.Value.Type, - "value": value, + "id": metadataEntryFromVcd.ID, + "key": metadataEntryFromVcd.KeyValue.Key, + "readonly": metadataEntryFromVcd.IsReadOnly, + "domain": metadataEntryFromVcd.KeyValue.Domain, + "namespace": metadataEntryFromVcd.KeyValue.Namespace, + "type": metadataEntryFromVcd.KeyValue.Value.Type, + "value": value, + "persistent": metadataEntryFromVcd.IsPersistent, } metadata[i] = metadataEntry } diff --git a/vcd/metadata_openapi_common_test.go b/vcd/metadata_openapi_common_test.go index f12a3b280..10028984a 100644 --- a/vcd/metadata_openapi_common_test.go +++ b/vcd/metadata_openapi_common_test.go @@ -35,7 +35,7 @@ func testOpenApiMetadataEntryCRUD(t *testing.T, resourceTemplate, resourceAddres debugPrintf("#[DEBUG] CONFIGURATION: %s", noMetadataHcl) params["FuncName"] = t.Name() + "Create" - params["Metadata"] = getOpenApiMetadataTestingHcl(1, 1, 1, 1, 1, 1) + params["Metadata"] = getOpenApiMetadataTestingHcl(1, 1, 1, 1, 1, 1, 1) createHcl := templateFill(resourceTemplate, params) debugPrintf("#[DEBUG] CONFIGURATION: %s", createHcl) @@ -44,7 +44,7 @@ func testOpenApiMetadataEntryCRUD(t *testing.T, resourceTemplate, resourceAddres debugPrintf("#[DEBUG] CONFIGURATION: %s", withDatasourceHcl) params["FuncName"] = t.Name() + "DeleteOneKey" - params["Metadata"] = getOpenApiMetadataTestingHcl(1, 1, 0, 1, 1, 1) + params["Metadata"] = getOpenApiMetadataTestingHcl(1, 1, 0, 1, 1, 1, 1) deleteOneKeyHcl := templateFill(resourceTemplate, params) debugPrintf("#[DEBUG] CONFIGURATION: %s", deleteOneKeyHcl) @@ -83,54 +83,58 @@ func testOpenApiMetadataEntryCRUD(t *testing.T, resourceTemplate, resourceAddres Taint: []string{resourceAddress}, // Forces re-creation to test Create with metadata. Check: resource.ComposeAggregateTestCheckFunc( resource.TestCheckResourceAttr(resourceAddress, "name", t.Name()), - resource.TestCheckResourceAttr(resourceAddress, "metadata_entry.#", "6"), - testCheckOpenApiMetadataEntrySetElemNestedAttrs(resourceAddress, "stringKey1", "stringValue1", types.OpenApiMetadataStringEntry, "TENANT", "", "false"), - testCheckOpenApiMetadataEntrySetElemNestedAttrs(resourceAddress, "numberKey1", "1", types.OpenApiMetadataNumberEntry, "TENANT", "", "false"), - testCheckOpenApiMetadataEntrySetElemNestedAttrs(resourceAddress, "boolKey1", "false", types.OpenApiMetadataStringEntry, "TENANT", "", "false"), - testCheckOpenApiMetadataEntrySetElemNestedAttrs(resourceAddress, "readOnly1", "readOnly1", types.OpenApiMetadataStringEntry, "TENANT", "", "true"), - testCheckOpenApiMetadataEntrySetElemNestedAttrs(resourceAddress, "namespaced1", "namespaced1", types.OpenApiMetadataStringEntry, "TENANT", "namespace", "false"), - testCheckOpenApiMetadataEntrySetElemNestedAttrs(resourceAddress, "provider1", "provider1", types.OpenApiMetadataStringEntry, "PROVIDER", "", "false"), + resource.TestCheckResourceAttr(resourceAddress, "metadata_entry.#", "7"), + testCheckOpenApiMetadataEntrySetElemNestedAttrs(resourceAddress, "stringKey1", "stringValue1", types.OpenApiMetadataStringEntry, "TENANT", "", "false", "false"), + testCheckOpenApiMetadataEntrySetElemNestedAttrs(resourceAddress, "numberKey1", "1", types.OpenApiMetadataNumberEntry, "TENANT", "", "false", "false"), + testCheckOpenApiMetadataEntrySetElemNestedAttrs(resourceAddress, "boolKey1", "false", types.OpenApiMetadataStringEntry, "TENANT", "", "false", "false"), + testCheckOpenApiMetadataEntrySetElemNestedAttrs(resourceAddress, "readOnly1", "readOnly1", types.OpenApiMetadataStringEntry, "TENANT", "", "true", "false"), + testCheckOpenApiMetadataEntrySetElemNestedAttrs(resourceAddress, "namespaced1", "namespaced1", types.OpenApiMetadataStringEntry, "TENANT", "namespace", "false", "false"), + testCheckOpenApiMetadataEntrySetElemNestedAttrs(resourceAddress, "provider1", "provider1", types.OpenApiMetadataStringEntry, "PROVIDER", "", "false", "false"), + testCheckOpenApiMetadataEntrySetElemNestedAttrs(resourceAddress, "persistent1", "persistent1", types.OpenApiMetadataStringEntry, "TENANT", "", "false", "true"), ), }, { Config: withDatasourceHcl, Check: resource.ComposeAggregateTestCheckFunc( resource.TestCheckResourceAttrPair(datasourceAddress, "id", resourceAddress, "id"), - resource.TestCheckResourceAttr(resourceAddress, "metadata_entry.#", "6"), - resource.TestCheckResourceAttr(datasourceAddress, "metadata_entry.#", "6"), - testCheckOpenApiMetadataEntrySetElemNestedAttrs(resourceAddress, "stringKey1", "stringValue1", types.OpenApiMetadataStringEntry, "TENANT", "", "false"), - testCheckOpenApiMetadataEntrySetElemNestedAttrs(resourceAddress, "numberKey1", "1", types.OpenApiMetadataNumberEntry, "TENANT", "", "false"), - testCheckOpenApiMetadataEntrySetElemNestedAttrs(resourceAddress, "boolKey1", "false", types.OpenApiMetadataBooleanEntry, "TENANT", "", "false"), - testCheckOpenApiMetadataEntrySetElemNestedAttrs(resourceAddress, "readOnly1", "readOnly1", types.OpenApiMetadataStringEntry, "TENANT", "", "true"), - testCheckOpenApiMetadataEntrySetElemNestedAttrs(resourceAddress, "namespaced1", "namespaced1", types.OpenApiMetadataStringEntry, "TENANT", "namespace", "false"), - testCheckOpenApiMetadataEntrySetElemNestedAttrs(resourceAddress, "provider1", "provider1", types.OpenApiMetadataStringEntry, "PROVIDER", "", "false"), + resource.TestCheckResourceAttr(resourceAddress, "metadata_entry.#", "7"), + resource.TestCheckResourceAttr(datasourceAddress, "metadata_entry.#", "7"), + testCheckOpenApiMetadataEntrySetElemNestedAttrs(resourceAddress, "stringKey1", "stringValue1", types.OpenApiMetadataStringEntry, "TENANT", "", "false", "false"), + testCheckOpenApiMetadataEntrySetElemNestedAttrs(resourceAddress, "numberKey1", "1", types.OpenApiMetadataNumberEntry, "TENANT", "", "false", "false"), + testCheckOpenApiMetadataEntrySetElemNestedAttrs(resourceAddress, "boolKey1", "false", types.OpenApiMetadataBooleanEntry, "TENANT", "", "false", "false"), + testCheckOpenApiMetadataEntrySetElemNestedAttrs(resourceAddress, "readOnly1", "readOnly1", types.OpenApiMetadataStringEntry, "TENANT", "", "true", "false"), + testCheckOpenApiMetadataEntrySetElemNestedAttrs(resourceAddress, "namespaced1", "namespaced1", types.OpenApiMetadataStringEntry, "TENANT", "namespace", "false", "false"), + testCheckOpenApiMetadataEntrySetElemNestedAttrs(resourceAddress, "provider1", "provider1", types.OpenApiMetadataStringEntry, "PROVIDER", "", "false", "false"), + testCheckOpenApiMetadataEntrySetElemNestedAttrs(resourceAddress, "persistent1", "persistent1", types.OpenApiMetadataStringEntry, "TENANT", "", "false", "true"), ), }, { Config: deleteOneKeyHcl, Check: resource.ComposeAggregateTestCheckFunc( resource.TestCheckResourceAttr(resourceAddress, "name", t.Name()), - resource.TestCheckResourceAttr(resourceAddress, "metadata_entry.#", "5"), + resource.TestCheckResourceAttr(resourceAddress, "metadata_entry.#", "6"), // The bool is deleted - testCheckOpenApiMetadataEntrySetElemNestedAttrs(resourceAddress, "stringKey1", "stringValue1", types.OpenApiMetadataStringEntry, "TENANT", "", "false"), - testCheckOpenApiMetadataEntrySetElemNestedAttrs(resourceAddress, "numberKey1", "1", types.OpenApiMetadataNumberEntry, "TENANT", "", "false"), - testCheckOpenApiMetadataEntrySetElemNestedAttrs(resourceAddress, "readOnly1", "readOnly1", types.OpenApiMetadataStringEntry, "TENANT", "", "true"), - testCheckOpenApiMetadataEntrySetElemNestedAttrs(resourceAddress, "namespaced1", "namespaced1", types.OpenApiMetadataStringEntry, "TENANT", "namespace", "false"), - testCheckOpenApiMetadataEntrySetElemNestedAttrs(resourceAddress, "provider1", "provider1", types.OpenApiMetadataStringEntry, "PROVIDER", "", "false"), + testCheckOpenApiMetadataEntrySetElemNestedAttrs(resourceAddress, "stringKey1", "stringValue1", types.OpenApiMetadataStringEntry, "TENANT", "", "false", "false"), + testCheckOpenApiMetadataEntrySetElemNestedAttrs(resourceAddress, "numberKey1", "1", types.OpenApiMetadataNumberEntry, "TENANT", "", "false", "false"), + testCheckOpenApiMetadataEntrySetElemNestedAttrs(resourceAddress, "readOnly1", "readOnly1", types.OpenApiMetadataStringEntry, "TENANT", "", "true", "false"), + testCheckOpenApiMetadataEntrySetElemNestedAttrs(resourceAddress, "namespaced1", "namespaced1", types.OpenApiMetadataStringEntry, "TENANT", "namespace", "false", "false"), + testCheckOpenApiMetadataEntrySetElemNestedAttrs(resourceAddress, "provider1", "provider1", types.OpenApiMetadataStringEntry, "PROVIDER", "", "false", "false"), + testCheckOpenApiMetadataEntrySetElemNestedAttrs(resourceAddress, "persistent1", "persistent1", types.OpenApiMetadataStringEntry, "TENANT", "", "false", "true"), ), }, { Config: updateHcl, Check: resource.ComposeAggregateTestCheckFunc( resource.TestCheckResourceAttr(resourceAddress, "name", t.Name()), - resource.TestCheckResourceAttr(resourceAddress, "metadata_entry.#", "5"), + resource.TestCheckResourceAttr(resourceAddress, "metadata_entry.#", "6"), // Updated value: - testCheckOpenApiMetadataEntrySetElemNestedAttrs(resourceAddress, "stringKey1", "stringValueUpdated1", types.OpenApiMetadataStringEntry, "TENANT", "", "false"), + testCheckOpenApiMetadataEntrySetElemNestedAttrs(resourceAddress, "stringKey1", "stringValueUpdated1", types.OpenApiMetadataStringEntry, "TENANT", "", "false", "false"), // Not updated values: - testCheckOpenApiMetadataEntrySetElemNestedAttrs(resourceAddress, "numberKey1", "1", types.OpenApiMetadataNumberEntry, "TENANT", "", "false"), - testCheckOpenApiMetadataEntrySetElemNestedAttrs(resourceAddress, "readOnly1", "readOnly1", types.OpenApiMetadataStringEntry, "TENANT", "", "true"), - testCheckOpenApiMetadataEntrySetElemNestedAttrs(resourceAddress, "namespaced1", "namespaced1", types.OpenApiMetadataStringEntry, "TENANT", "namespace", "false"), - testCheckOpenApiMetadataEntrySetElemNestedAttrs(resourceAddress, "provider1", "provider1", types.OpenApiMetadataStringEntry, "PROVIDER", "", "false"), + testCheckOpenApiMetadataEntrySetElemNestedAttrs(resourceAddress, "numberKey1", "1", types.OpenApiMetadataNumberEntry, "TENANT", "", "false", "false"), + testCheckOpenApiMetadataEntrySetElemNestedAttrs(resourceAddress, "readOnly1", "readOnly1", types.OpenApiMetadataStringEntry, "TENANT", "", "true", "false"), + testCheckOpenApiMetadataEntrySetElemNestedAttrs(resourceAddress, "namespaced1", "namespaced1", types.OpenApiMetadataStringEntry, "TENANT", "namespace", "false", "false"), + testCheckOpenApiMetadataEntrySetElemNestedAttrs(resourceAddress, "provider1", "provider1", types.OpenApiMetadataStringEntry, "PROVIDER", "", "false", "false"), + testCheckOpenApiMetadataEntrySetElemNestedAttrs(resourceAddress, "persistent1", "persistent1", types.OpenApiMetadataStringEntry, "TENANT", "", "false", "true"), ), }, { @@ -145,7 +149,7 @@ func testOpenApiMetadataEntryCRUD(t *testing.T, resourceTemplate, resourceAddres Check: resource.ComposeAggregateTestCheckFunc( resource.TestCheckResourceAttr(resourceAddress, "name", t.Name()), resource.TestCheckResourceAttr(resourceAddress, "metadata_entry.#", "1"), - testCheckOpenApiMetadataEntrySetElemNestedAttrs(resourceAddress, "defaultKey", "defaultValue", types.OpenApiMetadataStringEntry, "TENANT", "", "false"), + testCheckOpenApiMetadataEntrySetElemNestedAttrs(resourceAddress, "defaultKey", "defaultValue", types.OpenApiMetadataStringEntry, "TENANT", "", "false", "false"), ), }, }, @@ -154,31 +158,34 @@ func testOpenApiMetadataEntryCRUD(t *testing.T, resourceTemplate, resourceAddres } // getOpenApiMetadataTestingHcl gets valid metadata entries to inject them into an HCL for testing -func getOpenApiMetadataTestingHcl(stringEntries, numberEntries, boolEntries, readonlyEntries, namespacedEntries, providerEntries int) string { +func getOpenApiMetadataTestingHcl(stringEntries, numberEntries, boolEntries, readonlyEntries, namespacedEntries, providerEntries, persistentEntries int) string { hcl := "" for i := 1; i <= stringEntries; i++ { - hcl += getOpenApiMetadataEntryHcl(fmt.Sprintf("stringKey%d", i), fmt.Sprintf("stringValue%d", i), types.OpenApiMetadataStringEntry, "TENANT", "", "false") + hcl += getOpenApiMetadataEntryHcl(fmt.Sprintf("stringKey%d", i), fmt.Sprintf("stringValue%d", i), types.OpenApiMetadataStringEntry, "TENANT", "", "false", "false") } for i := 1; i <= numberEntries; i++ { - hcl += getOpenApiMetadataEntryHcl(fmt.Sprintf("numberKey%d", i), fmt.Sprintf("%d", i), types.OpenApiMetadataNumberEntry, "TENANT", "", "false") + hcl += getOpenApiMetadataEntryHcl(fmt.Sprintf("numberKey%d", i), fmt.Sprintf("%d", i), types.OpenApiMetadataNumberEntry, "TENANT", "", "false", "false") } for i := 1; i <= boolEntries; i++ { - hcl += getOpenApiMetadataEntryHcl(fmt.Sprintf("boolKey%d", i), strconv.FormatBool(i%2 == 0), types.OpenApiMetadataBooleanEntry, "TENANT", "", "false") + hcl += getOpenApiMetadataEntryHcl(fmt.Sprintf("boolKey%d", i), strconv.FormatBool(i%2 == 0), types.OpenApiMetadataBooleanEntry, "TENANT", "", "false", "false") } for i := 1; i <= readonlyEntries; i++ { - hcl += getOpenApiMetadataEntryHcl(fmt.Sprintf("readOnly%d", i), fmt.Sprintf("readOnly%d", i), types.OpenApiMetadataStringEntry, "TENANT", "", "true") + hcl += getOpenApiMetadataEntryHcl(fmt.Sprintf("readOnly%d", i), fmt.Sprintf("readOnly%d", i), types.OpenApiMetadataStringEntry, "TENANT", "", "true", "false") } for i := 1; i <= namespacedEntries; i++ { - hcl += getOpenApiMetadataEntryHcl(fmt.Sprintf("namespaced%d", i), fmt.Sprintf("namespaced%d", i), types.OpenApiMetadataStringEntry, "TENANT", "namespace", "false") + hcl += getOpenApiMetadataEntryHcl(fmt.Sprintf("namespaced%d", i), fmt.Sprintf("namespaced%d", i), types.OpenApiMetadataStringEntry, "TENANT", "namespace", "false", "false") } for i := 1; i <= providerEntries; i++ { - hcl += getOpenApiMetadataEntryHcl(fmt.Sprintf("provider%d", i), fmt.Sprintf("provider%d", i), types.OpenApiMetadataStringEntry, "PROVIDER", "", "false") + hcl += getOpenApiMetadataEntryHcl(fmt.Sprintf("provider%d", i), fmt.Sprintf("provider%d", i), types.OpenApiMetadataStringEntry, "PROVIDER", "", "false", "false") + } + for i := 1; i <= persistentEntries; i++ { + hcl += getOpenApiMetadataEntryHcl(fmt.Sprintf("persistent%d", i), fmt.Sprintf("persistent%d", i), types.OpenApiMetadataStringEntry, "TENANT", "", "false", "true") } return hcl } -func getOpenApiMetadataEntryHcl(key, value, typedValue, domain, namespace, readonly string) string { +func getOpenApiMetadataEntryHcl(key, value, typedValue, domain, namespace, readonly, persistent string) string { hclNamespace := "" if namespace != "" { hclNamespace = `namespace = "` + namespace + `"` @@ -192,19 +199,21 @@ func getOpenApiMetadataEntryHcl(key, value, typedValue, domain, namespace, reado domain = "` + domain + `" ` + hclNamespace + ` readonly = ` + readonly + ` + persistent = ` + persistent + ` }` } // testCheckOpenApiMetadataEntrySetElemNestedAttrs asserts that a given metadata_entry has the expected input for the given resourceAddress. -func testCheckOpenApiMetadataEntrySetElemNestedAttrs(resourceAddress, expectedKey, expectedValue, expectedType, expectedDomain, expectedNamespace, expectedReadonly string) resource.TestCheckFunc { +func testCheckOpenApiMetadataEntrySetElemNestedAttrs(resourceAddress, expectedKey, expectedValue, expectedType, expectedDomain, expectedNamespace, expectedReadonly, expectedPersistent string) resource.TestCheckFunc { return resource.TestCheckTypeSetElemNestedAttrs(resourceAddress, "metadata_entry.*", map[string]string{ - "key": expectedKey, - "value": expectedValue, - "type": expectedType, - "domain": expectedDomain, - "readonly": expectedReadonly, - "namespace": expectedNamespace, + "key": expectedKey, + "value": expectedValue, + "type": expectedType, + "domain": expectedDomain, + "readonly": expectedReadonly, + "namespace": expectedNamespace, + "persistent": expectedPersistent, }, ) } diff --git a/website/docs/r/rde.html.markdown b/website/docs/r/rde.html.markdown index 3f883d8ef..d9934bb65 100644 --- a/website/docs/r/rde.html.markdown +++ b/website/docs/r/rde.html.markdown @@ -196,6 +196,8 @@ The `metadata_entry` is a set of metadata entries that have the following struct * `type` - (Optional) Type of this metadata entry. One of: `StringEntry`, `NumberEntry`, `BoolEntry`. Defaults to `StringEntry`. * `domain` - (Optional) Only meaningful for providers. Allows them to share entries with their tenants. Currently, accepted values are: `TENANT`, `PROVIDER`. Defaults to `TENANT`. * `readonly` - (Optional) `true` if the metadata entry is read only. Defaults to `false`. +* `persistent` - (Optional) `true` if the metadata is persistent. Persistent entries can be copied over on some entity operation + (e.g. Creating a copy of a VDC, capturing a vApp to a template, instantiating a catalog item as a VM...). Defaults to `false`. * `id` - (Computed) Read-only identifier for this metadata entry. Example: From 5d0464d9d18af77a11e10650629268313b2b6bc1 Mon Sep 17 00:00:00 2001 From: abarreiro Date: Tue, 21 Nov 2023 13:05:52 +0100 Subject: [PATCH 06/39] Fix Signed-off-by: abarreiro --- vcd/metadata_openapi.go | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/vcd/metadata_openapi.go b/vcd/metadata_openapi.go index d3655e452..b59a5fbab 100644 --- a/vcd/metadata_openapi.go +++ b/vcd/metadata_openapi.go @@ -5,6 +5,7 @@ import ( "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/validation" "github.com/vmware/go-vcloud-director/v2/types/v56" + "reflect" "strconv" ) @@ -188,7 +189,7 @@ func getMetadataOperations(oldMetadata []interface{}, newMetadata []interface{}) metadataToUpdate := map[string]types.OpenApiMetadataEntry{} for newKey, newEntry := range newMetadataEntries { if oldEntry, ok := oldMetadataEntries[newKey]; ok { - if oldEntry.KeyValue.Value == newEntry.KeyValue.Value.Value { + if reflect.DeepEqual(oldEntry, newEntry) { continue } metadataToUpdate[newKey] = newEntry @@ -197,7 +198,9 @@ func getMetadataOperations(oldMetadata []interface{}, newMetadata []interface{}) var metadataToCreate []types.OpenApiMetadataEntry for newKey, newEntry := range newMetadataEntries { - if _, ok := metadataToUpdate[newKey]; !ok { + _, alreadyExisting := oldMetadataEntries[newKey] + _, beingUpdated := metadataToUpdate[newKey] + if !alreadyExisting && !beingUpdated { metadataToCreate = append(metadataToCreate, newEntry) } } From e988d47980efa16b9943dcaa3ffa7f9e12ad939f Mon Sep 17 00:00:00 2001 From: abarreiro Date: Tue, 21 Nov 2023 13:52:55 +0100 Subject: [PATCH 07/39] # Signed-off-by: abarreiro --- vcd/metadata_openapi.go | 1 - 1 file changed, 1 deletion(-) diff --git a/vcd/metadata_openapi.go b/vcd/metadata_openapi.go index b59a5fbab..6378e925d 100644 --- a/vcd/metadata_openapi.go +++ b/vcd/metadata_openapi.go @@ -261,7 +261,6 @@ func updateOpenApiMetadataInState(d *schema.ResourceData, receiverObject openApi case types.OpenApiMetadataBooleanEntry: value = fmt.Sprintf("%t", metadataEntryFromVcd.KeyValue.Value.Value.(bool)) case types.OpenApiMetadataNumberEntry: - // OpenAPI doesn't value = fmt.Sprintf("%.0f", metadataEntryFromVcd.KeyValue.Value.Value.(float64)) case types.OpenApiMetadataStringEntry: value = metadataEntryFromVcd.KeyValue.Value.Value.(string) From 080af7240713ea5a396d18dce192470bb2184e5b Mon Sep 17 00:00:00 2001 From: abarreiro Date: Wed, 22 Nov 2023 10:18:02 +0100 Subject: [PATCH 08/39] Add note Signed-off-by: abarreiro --- website/docs/r/rde.html.markdown | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/website/docs/r/rde.html.markdown b/website/docs/r/rde.html.markdown index d9934bb65..1534507e8 100644 --- a/website/docs/r/rde.html.markdown +++ b/website/docs/r/rde.html.markdown @@ -189,6 +189,10 @@ resolved by anyone. ## Metadata +-> Due to Terraform limitations, `terraform plan` shows that all metadata entries will be deleted and re-created when performing +a change in of one or more of them, but applying it will respect every non-changed metadata entry by not performing any modification on +these. + The `metadata_entry` is a set of metadata entries that have the following structure: * `key` - (Required) Key of this metadata entry. @@ -217,11 +221,11 @@ resource "vcd_rde" "my-rde" { readonly = true } metadata_entry { - key = "bar" - type = "NumberEntry" - value = "42" - domain = "TENANT" - readonly = true + key = "bar" + type = "NumberEntry" + value = "42" + domain = "TENANT" + permanent = true } } ``` From a7352a3350284e11dcec35f33f38314dd2d90dfe Mon Sep 17 00:00:00 2001 From: abarreiro Date: Wed, 22 Nov 2023 11:05:52 +0100 Subject: [PATCH 09/39] Fixes Signed-off-by: abarreiro --- vcd/metadata_openapi.go | 53 +++++++++++++++++++------------- website/docs/r/rde.html.markdown | 1 + 2 files changed, 32 insertions(+), 22 deletions(-) diff --git a/vcd/metadata_openapi.go b/vcd/metadata_openapi.go index 6378e925d..75b4e366d 100644 --- a/vcd/metadata_openapi.go +++ b/vcd/metadata_openapi.go @@ -7,6 +7,7 @@ import ( "github.com/vmware/go-vcloud-director/v2/types/v56" "reflect" "strconv" + "strings" ) // openApiMetadataEntryDatasourceSchema returns the schema associated to the OpenAPI metadata_entry for a given data source. @@ -144,17 +145,22 @@ func createOrUpdateOpenApiMetadataEntryInVcd(d *schema.ResourceData, resource op return fmt.Errorf("could not calculate the needed metadata operations: %s", err) } - for _, metadataKey := range metadataToDelete { - err := resource.DeleteMetadata(metadataKey) + // getMetadataOperations gets keys with namespaces. This function retrieves only the key. + getKey := func(namespacedKey string) string { + return strings.Split(namespacedKey, ",namespace=")[0] + } + + for _, namespacedMetadataKey := range metadataToDelete { + err := resource.DeleteMetadata(getKey(namespacedMetadataKey)) if err != nil { - return fmt.Errorf("error deleting metadata: %s", err) + return fmt.Errorf("error deleting metadata with %s: %s", namespacedMetadataKey, err) } } - for metadataKey, metadataEntry := range metadataToUpdate { - _, err := resource.UpdateMetadata(metadataKey, metadataEntry.KeyValue.Value.Value) + for namespacedMetadataKey, metadataEntry := range metadataToUpdate { + _, err := resource.UpdateMetadata(getKey(namespacedMetadataKey), metadataEntry.KeyValue.Value.Value) if err != nil { - return fmt.Errorf("error updating metadata: %s", err) + return fmt.Errorf("error updating metadata with %s: %s", namespacedMetadataKey, err) } } @@ -180,26 +186,26 @@ func getMetadataOperations(oldMetadata []interface{}, newMetadata []interface{}) } var metadataToRemove []string - for oldKey := range oldMetadataEntries { - if _, ok := newMetadataEntries[oldKey]; !ok { - metadataToRemove = append(metadataToRemove, oldKey) + for oldNamespacedKey := range oldMetadataEntries { + if _, ok := newMetadataEntries[oldNamespacedKey]; !ok { + metadataToRemove = append(metadataToRemove, oldNamespacedKey) } } metadataToUpdate := map[string]types.OpenApiMetadataEntry{} - for newKey, newEntry := range newMetadataEntries { - if oldEntry, ok := oldMetadataEntries[newKey]; ok { + for newNamespacedKey, newEntry := range newMetadataEntries { + if oldEntry, ok := oldMetadataEntries[newNamespacedKey]; ok { if reflect.DeepEqual(oldEntry, newEntry) { continue } - metadataToUpdate[newKey] = newEntry + metadataToUpdate[newNamespacedKey] = newEntry } } var metadataToCreate []types.OpenApiMetadataEntry - for newKey, newEntry := range newMetadataEntries { - _, alreadyExisting := oldMetadataEntries[newKey] - _, beingUpdated := metadataToUpdate[newKey] + for newNamespacedKey, newEntry := range newMetadataEntries { + _, alreadyExisting := oldMetadataEntries[newNamespacedKey] + _, beingUpdated := metadataToUpdate[newNamespacedKey] if !alreadyExisting && !beingUpdated { metadataToCreate = append(metadataToCreate, newEntry) } @@ -208,7 +214,8 @@ func getMetadataOperations(oldMetadata []interface{}, newMetadata []interface{}) return metadataToCreate, metadataToUpdate, metadataToRemove, nil } -// getOpenApiMetadataKeySet converts the input metadata attribute from Terraform state to a map composed by metadata keys and their values. +// getOpenApiMetadataEntryMap converts the input metadata attribute from Terraform state to a map composed by metadata +// namespaced keys and their values. func getOpenApiMetadataEntryMap(metadataAttribute []interface{}) (map[string]types.OpenApiMetadataEntry, error) { metadataMap := map[string]types.OpenApiMetadataEntry{} for _, rawItem := range metadataAttribute { @@ -224,7 +231,13 @@ func getOpenApiMetadataEntryMap(metadataAttribute []interface{}) (map[string]typ return nil, fmt.Errorf("error parsing the 'value' attribute '%s' from state: %s", metadataEntry["value"].(string), err) } - metadataMap[metadataEntry["key"].(string)] = types.OpenApiMetadataEntry{ + // In OpenAPI, metadata is namespaced, hence it is possible to have same keys but in different namespaces + namespacedKey := fmt.Sprintf("key=%s,namespace=%s", metadataEntry["key"].(string), namespace) + if _, ok := metadataMap[namespacedKey]; ok { + return nil, fmt.Errorf("metadata entry with %s already exists", namespacedKey) + } + + metadataMap[namespacedKey] = types.OpenApiMetadataEntry{ IsReadOnly: metadataEntry["readonly"].(bool), // It is always populated as it has a default value IsPersistent: metadataEntry["persistent"].(bool), // It is always populated as it has a default value KeyValue: types.OpenApiMetadataKeyValue{ @@ -241,7 +254,7 @@ func getOpenApiMetadataEntryMap(metadataAttribute []interface{}) (map[string]typ return metadataMap, nil } -// updateOpenApiMetadataInState updates metadata and metadata_entry in the Terraform state for the given receiver object. +// updateOpenApiMetadataInState updates metadata_entry in the Terraform state for the given receiver object. // This can be done as both are Computed, for compatibility reasons. func updateOpenApiMetadataInState(d *schema.ResourceData, receiverObject openApiMetadataCompatible) error { allMetadata, err := receiverObject.GetMetadata() @@ -249,10 +262,6 @@ func updateOpenApiMetadataInState(d *schema.ResourceData, receiverObject openApi return err } - if len(allMetadata) == 0 { - return nil - } - metadata := make([]interface{}, len(allMetadata)) for i, metadataEntryFromVcd := range allMetadata { // We need to set the correct type, otherwise saving the state will fail diff --git a/website/docs/r/rde.html.markdown b/website/docs/r/rde.html.markdown index 1534507e8..58d93165c 100644 --- a/website/docs/r/rde.html.markdown +++ b/website/docs/r/rde.html.markdown @@ -198,6 +198,7 @@ The `metadata_entry` is a set of metadata entries that have the following struct * `key` - (Required) Key of this metadata entry. * `value` - (Required) Value of this metadata entry. * `type` - (Optional) Type of this metadata entry. One of: `StringEntry`, `NumberEntry`, `BoolEntry`. Defaults to `StringEntry`. +* `namespace` - (Optional) Namespace of the metadata entry. Allows having multiple entries with same key in different namespaces. * `domain` - (Optional) Only meaningful for providers. Allows them to share entries with their tenants. Currently, accepted values are: `TENANT`, `PROVIDER`. Defaults to `TENANT`. * `readonly` - (Optional) `true` if the metadata entry is read only. Defaults to `false`. * `persistent` - (Optional) `true` if the metadata is persistent. Persistent entries can be copied over on some entity operation From 41304247a386b70b03fab21269f9c9fa4405d691 Mon Sep 17 00:00:00 2001 From: abarreiro Date: Wed, 22 Nov 2023 12:12:42 +0100 Subject: [PATCH 10/39] Fix Signed-off-by: abarreiro --- go.mod | 2 +- go.sum | 4 +-- vcd/metadata_openapi.go | 30 ++++++++++++------ vcd/metadata_openapi_common_test.go | 48 +++++++++++++++++++---------- 4 files changed, 56 insertions(+), 28 deletions(-) diff --git a/go.mod b/go.mod index c1cc2e1d0..60d813aef 100644 --- a/go.mod +++ b/go.mod @@ -66,4 +66,4 @@ require ( google.golang.org/protobuf v1.31.0 // indirect ) -replace github.com/vmware/go-vcloud-director/v2 => github.com/adambarreiro/go-vcloud-director/v2 v2.17.0-alpha.1.0.20231120104249-9e49febd72de +replace github.com/vmware/go-vcloud-director/v2 => github.com/adambarreiro/go-vcloud-director/v2 v2.17.0-alpha.1.0.20231122110403-a2835e0d3940 diff --git a/go.sum b/go.sum index 20618fa53..627ee509c 100644 --- a/go.sum +++ b/go.sum @@ -3,8 +3,8 @@ github.com/Microsoft/go-winio v0.6.1 h1:9/kr64B9VUZrLm5YYwbGtUJnMgqWVOdUAXu6Migc github.com/ProtonMail/go-crypto v0.0.0-20230717121422-5aa5874ade95 h1:KLq8BE0KwCL+mmXnjLWEAOYO+2l2AE4YMmqG1ZpZHBs= github.com/ProtonMail/go-crypto v0.0.0-20230717121422-5aa5874ade95/go.mod h1:EjAoLdwvbIOoOQr3ihjnSoLZRtE8azugULFRteWMNc0= github.com/acomagu/bufpipe v1.0.4 h1:e3H4WUzM3npvo5uv95QuJM3cQspFNtFBzvJ2oNjKIDQ= -github.com/adambarreiro/go-vcloud-director/v2 v2.17.0-alpha.1.0.20231120104249-9e49febd72de h1:pRnEzaUtaEdRpVLsJbDNGIKJ2Hr0DejmtfqoLkSrd7Y= -github.com/adambarreiro/go-vcloud-director/v2 v2.17.0-alpha.1.0.20231120104249-9e49febd72de/go.mod h1:QPxGFgrUcSyzy9IlpwDE4UNT3tsOy2047tJOPEJ4nlw= +github.com/adambarreiro/go-vcloud-director/v2 v2.17.0-alpha.1.0.20231122110403-a2835e0d3940 h1:3r8zHFAK+p+gTkcYWqlm59MSEwFZTk307snLYFbxLDw= +github.com/adambarreiro/go-vcloud-director/v2 v2.17.0-alpha.1.0.20231122110403-a2835e0d3940/go.mod h1:QPxGFgrUcSyzy9IlpwDE4UNT3tsOy2047tJOPEJ4nlw= github.com/agext/levenshtein v1.2.2 h1:0S/Yg6LYmFJ5stwQeRp6EeOcCbj7xiqQSdNelsXvaqE= github.com/agext/levenshtein v1.2.2/go.mod h1:JEDfjyjHDjOF/1e4FlBE/PkbqA9OfWu2ki2W0IB5558= github.com/apparentlymart/go-textseg/v12 v12.0.0/go.mod h1:S/4uRK2UtaQttw1GenVJEynmyUenKwP++x/+DdGV/Ec= diff --git a/vcd/metadata_openapi.go b/vcd/metadata_openapi.go index 75b4e366d..d1b71ac43 100644 --- a/vcd/metadata_openapi.go +++ b/vcd/metadata_openapi.go @@ -126,10 +126,10 @@ func openApiMetadataEntryResourceSchema(resourceType string) *schema.Schema { // openApiMetadataCompatible allows to consider all structs that implement OpenAPI metadata handling to be the same type type openApiMetadataCompatible interface { GetMetadata() ([]*types.OpenApiMetadataEntry, error) - GetMetadataByKey(key string) (*types.OpenApiMetadataEntry, error) + GetMetadataByKey(namespace, key string) (*types.OpenApiMetadataEntry, error) AddMetadata(metadataEntry types.OpenApiMetadataEntry) (*types.OpenApiMetadataEntry, error) - UpdateMetadata(key string, value interface{}) (*types.OpenApiMetadataEntry, error) - DeleteMetadata(key string) error + UpdateMetadata(namespace, key string, value interface{}) (*types.OpenApiMetadataEntry, error) + DeleteMetadata(namespace, key string) error } // createOrUpdateOpenApiMetadataEntryInVcd creates or updates OpenAPI metadata entries in VCD for the given resource, only if the attribute @@ -145,20 +145,32 @@ func createOrUpdateOpenApiMetadataEntryInVcd(d *schema.ResourceData, resource op return fmt.Errorf("could not calculate the needed metadata operations: %s", err) } - // getMetadataOperations gets keys with namespaces. This function retrieves only the key. - getKey := func(namespacedKey string) string { - return strings.Split(namespacedKey, ",namespace=")[0] + // getMetadataOperations gets keys with namespaces separated by %%%. This function retrieves both separated. + getKeyAndNamespace := func(namespacedKey string) (string, string, error) { + r := strings.Split(namespacedKey, "%%%") + if len(r) == 2 { + return r[0], r[1], nil + } + return "", "", fmt.Errorf("bad formatting of metadata map %s, this is a provider error", namespacedKey) } for _, namespacedMetadataKey := range metadataToDelete { - err := resource.DeleteMetadata(getKey(namespacedMetadataKey)) + key, namespace, err := getKeyAndNamespace(namespacedMetadataKey) + if err != nil { + return err + } + err = resource.DeleteMetadata(namespace, key) if err != nil { return fmt.Errorf("error deleting metadata with %s: %s", namespacedMetadataKey, err) } } for namespacedMetadataKey, metadataEntry := range metadataToUpdate { - _, err := resource.UpdateMetadata(getKey(namespacedMetadataKey), metadataEntry.KeyValue.Value.Value) + key, namespace, err := getKeyAndNamespace(namespacedMetadataKey) + if err != nil { + return err + } + _, err = resource.UpdateMetadata(namespace, key, metadataEntry.KeyValue.Value.Value) if err != nil { return fmt.Errorf("error updating metadata with %s: %s", namespacedMetadataKey, err) } @@ -232,7 +244,7 @@ func getOpenApiMetadataEntryMap(metadataAttribute []interface{}) (map[string]typ } // In OpenAPI, metadata is namespaced, hence it is possible to have same keys but in different namespaces - namespacedKey := fmt.Sprintf("key=%s,namespace=%s", metadataEntry["key"].(string), namespace) + namespacedKey := fmt.Sprintf("%s%%%%%%%s", namespace, metadataEntry["key"].(string)) if _, ok := metadataMap[namespacedKey]; ok { return nil, fmt.Errorf("metadata entry with %s already exists", namespacedKey) } diff --git a/vcd/metadata_openapi_common_test.go b/vcd/metadata_openapi_common_test.go index 10028984a..a472059ae 100644 --- a/vcd/metadata_openapi_common_test.go +++ b/vcd/metadata_openapi_common_test.go @@ -24,6 +24,13 @@ func testOpenApiMetadataEntryCRUD(t *testing.T, resourceTemplate, resourceAddres "Name": t.Name(), } + outputHcl := ` + output "metadata_id" { + value = tolist(` + resourceAddress + `.metadata_entry)[0].id + } + ` + templateWithOutput := resourceTemplate + outputHcl + for extraParam, extraParamValue := range extraParams { params[extraParam] = extraParamValue } @@ -35,22 +42,22 @@ func testOpenApiMetadataEntryCRUD(t *testing.T, resourceTemplate, resourceAddres debugPrintf("#[DEBUG] CONFIGURATION: %s", noMetadataHcl) params["FuncName"] = t.Name() + "Create" - params["Metadata"] = getOpenApiMetadataTestingHcl(1, 1, 1, 1, 1, 1, 1) - createHcl := templateFill(resourceTemplate, params) + params["Metadata"] = getOpenApiMetadataTestingHcl(1, 1, 1, 1, 2, 1, 1) + createHcl := templateFill(templateWithOutput, params) debugPrintf("#[DEBUG] CONFIGURATION: %s", createHcl) params["FuncName"] = t.Name() + "WithDatasource" - withDatasourceHcl := templateFill(datasourceTemplate+"\n# skip-binary-test\n"+resourceTemplate, params) + withDatasourceHcl := templateFill(datasourceTemplate+"\n# skip-binary-test\n"+templateWithOutput, params) debugPrintf("#[DEBUG] CONFIGURATION: %s", withDatasourceHcl) params["FuncName"] = t.Name() + "DeleteOneKey" - params["Metadata"] = getOpenApiMetadataTestingHcl(1, 1, 0, 1, 1, 1, 1) - deleteOneKeyHcl := templateFill(resourceTemplate, params) + params["Metadata"] = getOpenApiMetadataTestingHcl(1, 1, 0, 1, 2, 1, 1) + deleteOneKeyHcl := templateFill(templateWithOutput, params) debugPrintf("#[DEBUG] CONFIGURATION: %s", deleteOneKeyHcl) params["FuncName"] = t.Name() + "Update" params["Metadata"] = strings.NewReplacer("stringValue", "stringValueUpdated").Replace(params["Metadata"].(string)) - updateHcl := templateFill(resourceTemplate, params) + updateHcl := templateFill(templateWithOutput, params) debugPrintf("#[DEBUG] CONFIGURATION: %s", updateHcl) params["FuncName"] = t.Name() + "Delete" @@ -68,6 +75,9 @@ func testOpenApiMetadataEntryCRUD(t *testing.T, resourceTemplate, resourceAddres return } + // This is used to validate that metadata IDs don't change despite of an update/delete. + //cachedId := testCachedFieldValue{} + resource.Test(t, resource.TestCase{ ProviderFactories: testAccProviders, Steps: []resource.TestStep{ @@ -82,13 +92,15 @@ func testOpenApiMetadataEntryCRUD(t *testing.T, resourceTemplate, resourceAddres Config: createHcl, Taint: []string{resourceAddress}, // Forces re-creation to test Create with metadata. Check: resource.ComposeAggregateTestCheckFunc( + //cachedId.cacheTestResourceFieldValue("output.metadata_id",), resource.TestCheckResourceAttr(resourceAddress, "name", t.Name()), - resource.TestCheckResourceAttr(resourceAddress, "metadata_entry.#", "7"), + resource.TestCheckResourceAttr(resourceAddress, "metadata_entry.#", "8"), testCheckOpenApiMetadataEntrySetElemNestedAttrs(resourceAddress, "stringKey1", "stringValue1", types.OpenApiMetadataStringEntry, "TENANT", "", "false", "false"), testCheckOpenApiMetadataEntrySetElemNestedAttrs(resourceAddress, "numberKey1", "1", types.OpenApiMetadataNumberEntry, "TENANT", "", "false", "false"), testCheckOpenApiMetadataEntrySetElemNestedAttrs(resourceAddress, "boolKey1", "false", types.OpenApiMetadataStringEntry, "TENANT", "", "false", "false"), testCheckOpenApiMetadataEntrySetElemNestedAttrs(resourceAddress, "readOnly1", "readOnly1", types.OpenApiMetadataStringEntry, "TENANT", "", "true", "false"), - testCheckOpenApiMetadataEntrySetElemNestedAttrs(resourceAddress, "namespaced1", "namespaced1", types.OpenApiMetadataStringEntry, "TENANT", "namespace", "false", "false"), + testCheckOpenApiMetadataEntrySetElemNestedAttrs(resourceAddress, "namespace", "namespace1", types.OpenApiMetadataStringEntry, "TENANT", "namespace1", "false", "false"), + testCheckOpenApiMetadataEntrySetElemNestedAttrs(resourceAddress, "namespace", "namespace2", types.OpenApiMetadataStringEntry, "TENANT", "namespace2", "false", "false"), testCheckOpenApiMetadataEntrySetElemNestedAttrs(resourceAddress, "provider1", "provider1", types.OpenApiMetadataStringEntry, "PROVIDER", "", "false", "false"), testCheckOpenApiMetadataEntrySetElemNestedAttrs(resourceAddress, "persistent1", "persistent1", types.OpenApiMetadataStringEntry, "TENANT", "", "false", "true"), ), @@ -97,13 +109,14 @@ func testOpenApiMetadataEntryCRUD(t *testing.T, resourceTemplate, resourceAddres Config: withDatasourceHcl, Check: resource.ComposeAggregateTestCheckFunc( resource.TestCheckResourceAttrPair(datasourceAddress, "id", resourceAddress, "id"), - resource.TestCheckResourceAttr(resourceAddress, "metadata_entry.#", "7"), - resource.TestCheckResourceAttr(datasourceAddress, "metadata_entry.#", "7"), + resource.TestCheckResourceAttr(resourceAddress, "metadata_entry.#", "8"), + resource.TestCheckResourceAttr(datasourceAddress, "metadata_entry.#", "8"), testCheckOpenApiMetadataEntrySetElemNestedAttrs(resourceAddress, "stringKey1", "stringValue1", types.OpenApiMetadataStringEntry, "TENANT", "", "false", "false"), testCheckOpenApiMetadataEntrySetElemNestedAttrs(resourceAddress, "numberKey1", "1", types.OpenApiMetadataNumberEntry, "TENANT", "", "false", "false"), testCheckOpenApiMetadataEntrySetElemNestedAttrs(resourceAddress, "boolKey1", "false", types.OpenApiMetadataBooleanEntry, "TENANT", "", "false", "false"), testCheckOpenApiMetadataEntrySetElemNestedAttrs(resourceAddress, "readOnly1", "readOnly1", types.OpenApiMetadataStringEntry, "TENANT", "", "true", "false"), - testCheckOpenApiMetadataEntrySetElemNestedAttrs(resourceAddress, "namespaced1", "namespaced1", types.OpenApiMetadataStringEntry, "TENANT", "namespace", "false", "false"), + testCheckOpenApiMetadataEntrySetElemNestedAttrs(resourceAddress, "namespace", "namespace1", types.OpenApiMetadataStringEntry, "TENANT", "namespace1", "false", "false"), + testCheckOpenApiMetadataEntrySetElemNestedAttrs(resourceAddress, "namespace", "namespace2", types.OpenApiMetadataStringEntry, "TENANT", "namespace2", "false", "false"), testCheckOpenApiMetadataEntrySetElemNestedAttrs(resourceAddress, "provider1", "provider1", types.OpenApiMetadataStringEntry, "PROVIDER", "", "false", "false"), testCheckOpenApiMetadataEntrySetElemNestedAttrs(resourceAddress, "persistent1", "persistent1", types.OpenApiMetadataStringEntry, "TENANT", "", "false", "true"), ), @@ -112,12 +125,13 @@ func testOpenApiMetadataEntryCRUD(t *testing.T, resourceTemplate, resourceAddres Config: deleteOneKeyHcl, Check: resource.ComposeAggregateTestCheckFunc( resource.TestCheckResourceAttr(resourceAddress, "name", t.Name()), - resource.TestCheckResourceAttr(resourceAddress, "metadata_entry.#", "6"), + resource.TestCheckResourceAttr(resourceAddress, "metadata_entry.#", "7"), // The bool is deleted testCheckOpenApiMetadataEntrySetElemNestedAttrs(resourceAddress, "stringKey1", "stringValue1", types.OpenApiMetadataStringEntry, "TENANT", "", "false", "false"), testCheckOpenApiMetadataEntrySetElemNestedAttrs(resourceAddress, "numberKey1", "1", types.OpenApiMetadataNumberEntry, "TENANT", "", "false", "false"), testCheckOpenApiMetadataEntrySetElemNestedAttrs(resourceAddress, "readOnly1", "readOnly1", types.OpenApiMetadataStringEntry, "TENANT", "", "true", "false"), - testCheckOpenApiMetadataEntrySetElemNestedAttrs(resourceAddress, "namespaced1", "namespaced1", types.OpenApiMetadataStringEntry, "TENANT", "namespace", "false", "false"), + testCheckOpenApiMetadataEntrySetElemNestedAttrs(resourceAddress, "namespace", "namespace1", types.OpenApiMetadataStringEntry, "TENANT", "namespace1", "false", "false"), + testCheckOpenApiMetadataEntrySetElemNestedAttrs(resourceAddress, "namespace", "namespace2", types.OpenApiMetadataStringEntry, "TENANT", "namespace2", "false", "false"), testCheckOpenApiMetadataEntrySetElemNestedAttrs(resourceAddress, "provider1", "provider1", types.OpenApiMetadataStringEntry, "PROVIDER", "", "false", "false"), testCheckOpenApiMetadataEntrySetElemNestedAttrs(resourceAddress, "persistent1", "persistent1", types.OpenApiMetadataStringEntry, "TENANT", "", "false", "true"), ), @@ -126,13 +140,14 @@ func testOpenApiMetadataEntryCRUD(t *testing.T, resourceTemplate, resourceAddres Config: updateHcl, Check: resource.ComposeAggregateTestCheckFunc( resource.TestCheckResourceAttr(resourceAddress, "name", t.Name()), - resource.TestCheckResourceAttr(resourceAddress, "metadata_entry.#", "6"), + resource.TestCheckResourceAttr(resourceAddress, "metadata_entry.#", "7"), // Updated value: testCheckOpenApiMetadataEntrySetElemNestedAttrs(resourceAddress, "stringKey1", "stringValueUpdated1", types.OpenApiMetadataStringEntry, "TENANT", "", "false", "false"), // Not updated values: testCheckOpenApiMetadataEntrySetElemNestedAttrs(resourceAddress, "numberKey1", "1", types.OpenApiMetadataNumberEntry, "TENANT", "", "false", "false"), testCheckOpenApiMetadataEntrySetElemNestedAttrs(resourceAddress, "readOnly1", "readOnly1", types.OpenApiMetadataStringEntry, "TENANT", "", "true", "false"), - testCheckOpenApiMetadataEntrySetElemNestedAttrs(resourceAddress, "namespaced1", "namespaced1", types.OpenApiMetadataStringEntry, "TENANT", "namespace", "false", "false"), + testCheckOpenApiMetadataEntrySetElemNestedAttrs(resourceAddress, "namespace", "namespace1", types.OpenApiMetadataStringEntry, "TENANT", "namespace1", "false", "false"), + testCheckOpenApiMetadataEntrySetElemNestedAttrs(resourceAddress, "namespace", "namespace2", types.OpenApiMetadataStringEntry, "TENANT", "namespace2", "false", "false"), testCheckOpenApiMetadataEntrySetElemNestedAttrs(resourceAddress, "provider1", "provider1", types.OpenApiMetadataStringEntry, "PROVIDER", "", "false", "false"), testCheckOpenApiMetadataEntrySetElemNestedAttrs(resourceAddress, "persistent1", "persistent1", types.OpenApiMetadataStringEntry, "TENANT", "", "false", "true"), ), @@ -173,7 +188,8 @@ func getOpenApiMetadataTestingHcl(stringEntries, numberEntries, boolEntries, rea hcl += getOpenApiMetadataEntryHcl(fmt.Sprintf("readOnly%d", i), fmt.Sprintf("readOnly%d", i), types.OpenApiMetadataStringEntry, "TENANT", "", "true", "false") } for i := 1; i <= namespacedEntries; i++ { - hcl += getOpenApiMetadataEntryHcl(fmt.Sprintf("namespaced%d", i), fmt.Sprintf("namespaced%d", i), types.OpenApiMetadataStringEntry, "TENANT", "namespace", "false", "false") + // This one shares the same key, as it is the goal of namespaces + hcl += getOpenApiMetadataEntryHcl("namespace", fmt.Sprintf("namespace%d", i), types.OpenApiMetadataStringEntry, "TENANT", fmt.Sprintf("namespace%d", i), "false", "false") } for i := 1; i <= providerEntries; i++ { hcl += getOpenApiMetadataEntryHcl(fmt.Sprintf("provider%d", i), fmt.Sprintf("provider%d", i), types.OpenApiMetadataStringEntry, "PROVIDER", "", "false", "false") From bc927bb2e31b238657fcc75d812256a36f6c36db Mon Sep 17 00:00:00 2001 From: abarreiro Date: Wed, 22 Nov 2023 12:19:20 +0100 Subject: [PATCH 11/39] Fix Signed-off-by: abarreiro --- vcd/metadata_openapi.go | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/vcd/metadata_openapi.go b/vcd/metadata_openapi.go index d1b71ac43..decd9420a 100644 --- a/vcd/metadata_openapi.go +++ b/vcd/metadata_openapi.go @@ -145,9 +145,10 @@ func createOrUpdateOpenApiMetadataEntryInVcd(d *schema.ResourceData, resource op return fmt.Errorf("could not calculate the needed metadata operations: %s", err) } - // getMetadataOperations gets keys with namespaces separated by %%%. This function retrieves both separated. + // getMetadataOperations retrieves keys and namespaces merged with a separator, this functions + // splits the values in two: namespace and key, separately. getKeyAndNamespace := func(namespacedKey string) (string, string, error) { - r := strings.Split(namespacedKey, "%%%") + r := strings.Split(namespacedKey, "%%%") // Separator used by getOpenApiMetadataEntryMap if len(r) == 2 { return r[0], r[1], nil } @@ -155,7 +156,7 @@ func createOrUpdateOpenApiMetadataEntryInVcd(d *schema.ResourceData, resource op } for _, namespacedMetadataKey := range metadataToDelete { - key, namespace, err := getKeyAndNamespace(namespacedMetadataKey) + namespace, key, err := getKeyAndNamespace(namespacedMetadataKey) if err != nil { return err } @@ -166,7 +167,7 @@ func createOrUpdateOpenApiMetadataEntryInVcd(d *schema.ResourceData, resource op } for namespacedMetadataKey, metadataEntry := range metadataToUpdate { - key, namespace, err := getKeyAndNamespace(namespacedMetadataKey) + namespace, key, err := getKeyAndNamespace(namespacedMetadataKey) if err != nil { return err } @@ -227,7 +228,7 @@ func getMetadataOperations(oldMetadata []interface{}, newMetadata []interface{}) } // getOpenApiMetadataEntryMap converts the input metadata attribute from Terraform state to a map composed by metadata -// namespaced keys and their values. +// namespaced keys (this is, namespace and key separated by '%%%') and their values. func getOpenApiMetadataEntryMap(metadataAttribute []interface{}) (map[string]types.OpenApiMetadataEntry, error) { metadataMap := map[string]types.OpenApiMetadataEntry{} for _, rawItem := range metadataAttribute { From 657ed4c8a33b87c44207881736b0c57ad3d96bac Mon Sep 17 00:00:00 2001 From: abarreiro Date: Wed, 22 Nov 2023 12:33:43 +0100 Subject: [PATCH 12/39] Improve test Signed-off-by: abarreiro --- vcd/metadata_openapi_common_test.go | 30 +++++++++++++++++++++++++++-- 1 file changed, 28 insertions(+), 2 deletions(-) diff --git a/vcd/metadata_openapi_common_test.go b/vcd/metadata_openapi_common_test.go index a472059ae..5ff07d915 100644 --- a/vcd/metadata_openapi_common_test.go +++ b/vcd/metadata_openapi_common_test.go @@ -5,6 +5,7 @@ package vcd import ( "fmt" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" + "github.com/hashicorp/terraform-plugin-sdk/v2/terraform" "github.com/vmware/go-vcloud-director/v2/types/v56" "strconv" "strings" @@ -24,6 +25,8 @@ func testOpenApiMetadataEntryCRUD(t *testing.T, resourceTemplate, resourceAddres "Name": t.Name(), } + // This output allows to perform some assertions on the ID inside a TypeSet, + // which is impossible to obtain otherwise. outputHcl := ` output "metadata_id" { value = tolist(` + resourceAddress + `.metadata_entry)[0].id @@ -76,7 +79,21 @@ func testOpenApiMetadataEntryCRUD(t *testing.T, resourceTemplate, resourceAddres } // This is used to validate that metadata IDs don't change despite of an update/delete. - //cachedId := testCachedFieldValue{} + cachedId := testCachedFieldValue{} + testIdsDontChange := func() func(s *terraform.State) error { + return func(s *terraform.State) error { + rs, ok := s.RootModule().Outputs["metadata_id"] + if !ok { + return fmt.Errorf("output 'metadata_id' not found") + } + + value := rs.String() + if cachedId.fieldValue != value || value == "" { + return fmt.Errorf("expected metadata_id to be '%s' but it changed to '%s'", cachedId.fieldValue, value) + } + return nil + } + } resource.Test(t, resource.TestCase{ ProviderFactories: testAccProviders, @@ -92,7 +109,14 @@ func testOpenApiMetadataEntryCRUD(t *testing.T, resourceTemplate, resourceAddres Config: createHcl, Taint: []string{resourceAddress}, // Forces re-creation to test Create with metadata. Check: resource.ComposeAggregateTestCheckFunc( - //cachedId.cacheTestResourceFieldValue("output.metadata_id",), + func(s *terraform.State) error { + rs, ok := s.RootModule().Outputs["metadata_id"] + if !ok { + return fmt.Errorf("output 'metadata_id' not found") + } + cachedId.fieldValue = rs.String() + return nil + }, resource.TestCheckResourceAttr(resourceAddress, "name", t.Name()), resource.TestCheckResourceAttr(resourceAddress, "metadata_entry.#", "8"), testCheckOpenApiMetadataEntrySetElemNestedAttrs(resourceAddress, "stringKey1", "stringValue1", types.OpenApiMetadataStringEntry, "TENANT", "", "false", "false"), @@ -134,6 +158,7 @@ func testOpenApiMetadataEntryCRUD(t *testing.T, resourceTemplate, resourceAddres testCheckOpenApiMetadataEntrySetElemNestedAttrs(resourceAddress, "namespace", "namespace2", types.OpenApiMetadataStringEntry, "TENANT", "namespace2", "false", "false"), testCheckOpenApiMetadataEntrySetElemNestedAttrs(resourceAddress, "provider1", "provider1", types.OpenApiMetadataStringEntry, "PROVIDER", "", "false", "false"), testCheckOpenApiMetadataEntrySetElemNestedAttrs(resourceAddress, "persistent1", "persistent1", types.OpenApiMetadataStringEntry, "TENANT", "", "false", "true"), + testIdsDontChange(), ), }, { @@ -150,6 +175,7 @@ func testOpenApiMetadataEntryCRUD(t *testing.T, resourceTemplate, resourceAddres testCheckOpenApiMetadataEntrySetElemNestedAttrs(resourceAddress, "namespace", "namespace2", types.OpenApiMetadataStringEntry, "TENANT", "namespace2", "false", "false"), testCheckOpenApiMetadataEntrySetElemNestedAttrs(resourceAddress, "provider1", "provider1", types.OpenApiMetadataStringEntry, "PROVIDER", "", "false", "false"), testCheckOpenApiMetadataEntrySetElemNestedAttrs(resourceAddress, "persistent1", "persistent1", types.OpenApiMetadataStringEntry, "TENANT", "", "false", "true"), + testIdsDontChange(), ), }, { From b54dd16a752e394d263c6ed220dce04a63ef16b1 Mon Sep 17 00:00:00 2001 From: abarreiro Date: Wed, 22 Nov 2023 12:35:26 +0100 Subject: [PATCH 13/39] self-review Signed-off-by: abarreiro --- vcd/metadata_openapi.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/vcd/metadata_openapi.go b/vcd/metadata_openapi.go index decd9420a..b842cf52e 100644 --- a/vcd/metadata_openapi.go +++ b/vcd/metadata_openapi.go @@ -145,7 +145,7 @@ func createOrUpdateOpenApiMetadataEntryInVcd(d *schema.ResourceData, resource op return fmt.Errorf("could not calculate the needed metadata operations: %s", err) } - // getMetadataOperations retrieves keys and namespaces merged with a separator, this functions + // getMetadataOperations retrieves keys and namespaces merged with a separator, this function // splits the values in two: namespace and key, separately. getKeyAndNamespace := func(namespacedKey string) (string, string, error) { r := strings.Split(namespacedKey, "%%%") // Separator used by getOpenApiMetadataEntryMap From 68215d15b378bd529cbaca20f61ef995310fc53f Mon Sep 17 00:00:00 2001 From: abarreiro Date: Wed, 22 Nov 2023 12:36:15 +0100 Subject: [PATCH 14/39] self-review Signed-off-by: abarreiro --- vcd/metadata_openapi.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/vcd/metadata_openapi.go b/vcd/metadata_openapi.go index b842cf52e..6b43e20d6 100644 --- a/vcd/metadata_openapi.go +++ b/vcd/metadata_openapi.go @@ -162,7 +162,7 @@ func createOrUpdateOpenApiMetadataEntryInVcd(d *schema.ResourceData, resource op } err = resource.DeleteMetadata(namespace, key) if err != nil { - return fmt.Errorf("error deleting metadata with %s: %s", namespacedMetadataKey, err) + return fmt.Errorf("error deleting metadata with namespace '%s' and key '%s': %s", namespace, key, err) } } @@ -173,7 +173,7 @@ func createOrUpdateOpenApiMetadataEntryInVcd(d *schema.ResourceData, resource op } _, err = resource.UpdateMetadata(namespace, key, metadataEntry.KeyValue.Value.Value) if err != nil { - return fmt.Errorf("error updating metadata with %s: %s", namespacedMetadataKey, err) + return fmt.Errorf("error updating metadata with namespace '%s' and key '%s': %s", namespace, key, err) } } From c6a0925967b5a643d99d731a24e632273a4cb6b6 Mon Sep 17 00:00:00 2001 From: abarreiro Date: Wed, 22 Nov 2023 12:39:04 +0100 Subject: [PATCH 15/39] self-review Signed-off-by: abarreiro --- vcd/metadata_openapi.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/vcd/metadata_openapi.go b/vcd/metadata_openapi.go index 6b43e20d6..2441fcad2 100644 --- a/vcd/metadata_openapi.go +++ b/vcd/metadata_openapi.go @@ -152,7 +152,7 @@ func createOrUpdateOpenApiMetadataEntryInVcd(d *schema.ResourceData, resource op if len(r) == 2 { return r[0], r[1], nil } - return "", "", fmt.Errorf("bad formatting of metadata map %s, this is a provider error", namespacedKey) + return "", "", fmt.Errorf("bad formatting of metadata map key %s, this is a provider error", namespacedKey) } for _, namespacedMetadataKey := range metadataToDelete { From b07d6634a20d4cfdfcab0e09d01b9cc349f5c5d5 Mon Sep 17 00:00:00 2001 From: abarreiro Date: Wed, 22 Nov 2023 12:42:30 +0100 Subject: [PATCH 16/39] self-review Signed-off-by: abarreiro --- vcd/metadata_openapi.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/vcd/metadata_openapi.go b/vcd/metadata_openapi.go index 2441fcad2..852fa5be2 100644 --- a/vcd/metadata_openapi.go +++ b/vcd/metadata_openapi.go @@ -140,12 +140,12 @@ func createOrUpdateOpenApiMetadataEntryInVcd(d *schema.ResourceData, resource op } oldRaw, newRaw := d.GetChange("metadata_entry") - metadataToAdd, metadataToUpdate, metadataToDelete, err := getMetadataOperations(oldRaw.(*schema.Set).List(), newRaw.(*schema.Set).List()) + metadataToAdd, metadataToUpdate, metadataToDelete, err := getOpenApiMetadataOperations(oldRaw.(*schema.Set).List(), newRaw.(*schema.Set).List()) if err != nil { return fmt.Errorf("could not calculate the needed metadata operations: %s", err) } - // getMetadataOperations retrieves keys and namespaces merged with a separator, this function + // getOpenApiMetadataOperations retrieves keys and namespaces merged with a separator, this function // splits the values in two: namespace and key, separately. getKeyAndNamespace := func(namespacedKey string) (string, string, error) { r := strings.Split(namespacedKey, "%%%") // Separator used by getOpenApiMetadataEntryMap @@ -186,9 +186,9 @@ func createOrUpdateOpenApiMetadataEntryInVcd(d *schema.ResourceData, resource op return nil } -// getMetadataOperations retrieves the metadata that needs to be added, to be updated and to be deleted depending +// getOpenApiMetadataOperations retrieves the metadata that needs to be added, to be updated and to be deleted depending // on the old and new attribute values from Terraform state. -func getMetadataOperations(oldMetadata []interface{}, newMetadata []interface{}) ([]types.OpenApiMetadataEntry, map[string]types.OpenApiMetadataEntry, []string, error) { +func getOpenApiMetadataOperations(oldMetadata []interface{}, newMetadata []interface{}) ([]types.OpenApiMetadataEntry, map[string]types.OpenApiMetadataEntry, []string, error) { oldMetadataEntries, err := getOpenApiMetadataEntryMap(oldMetadata) if err != nil { return nil, nil, nil, err From 1d0216699352c9b59eea9c9b2d68a16c3f8b9783 Mon Sep 17 00:00:00 2001 From: abarreiro Date: Wed, 22 Nov 2023 12:52:50 +0100 Subject: [PATCH 17/39] Remove version 10.4.0 checks Signed-off-by: abarreiro --- vcd/datasource_vcd_rde.go | 9 +++------ vcd/resource_vcd_rde.go | 22 ++++++---------------- vcd/resource_vcd_rde_test.go | 4 ---- 3 files changed, 9 insertions(+), 26 deletions(-) diff --git a/vcd/datasource_vcd_rde.go b/vcd/datasource_vcd_rde.go index ad47230a6..4929cb05f 100644 --- a/vcd/datasource_vcd_rde.go +++ b/vcd/datasource_vcd_rde.go @@ -84,12 +84,9 @@ func datasourceVcdRdeRead(_ context.Context, d *schema.ResourceData, meta interf dSet(d, "owner_user_id", rde.DefinedEntity.Owner.ID) } - // Metadata is only available since API v37.0 - if vcdClient.Client.APIVCDMaxVersionIs(">= 37.0") { - err = updateOpenApiMetadataInState(d, rde) - if err != nil { - return diag.Errorf("could not set metadata for the Runtime Defined Entity: %s", err) - } + err = updateOpenApiMetadataInState(d, rde) + if err != nil { + return diag.Errorf("could not set metadata for the Runtime Defined Entity: %s", err) } d.SetId(rde.DefinedEntity.ID) diff --git a/vcd/resource_vcd_rde.go b/vcd/resource_vcd_rde.go index 6cc4e165c..569368b5d 100644 --- a/vcd/resource_vcd_rde.go +++ b/vcd/resource_vcd_rde.go @@ -150,14 +150,9 @@ func resourceVcdRdeCreate(ctx context.Context, d *schema.ResourceData, meta inte } } - // Metadata is only supported since v37.0 - if vcdClient.Client.APIVCDMaxVersionIs(">= 37.0") { - err = createOrUpdateOpenApiMetadataEntryInVcd(d, rde) - if err != nil { - return diag.Errorf("could not create metadata for the Runtime Defined Entity: %s", err) - } - } else if _, ok := d.GetOk("metadata_entry"); ok { - return diag.Errorf("metadata_entry is only supported since VCD 10.4.0") + err = createOrUpdateOpenApiMetadataEntryInVcd(d, rde) + if err != nil { + return diag.Errorf("could not create metadata for the Runtime Defined Entity: %s", err) } return resourceVcdRdeRead(ctx, d, meta) @@ -339,14 +334,9 @@ func resourceVcdRdeUpdate(ctx context.Context, d *schema.ResourceData, meta inte } } - // Metadata is only supported since v37.0 - if vcdClient.Client.APIVCDMaxVersionIs(">= 37.0") { - err = createOrUpdateOpenApiMetadataEntryInVcd(d, rde) - if err != nil { - return diag.Errorf("could not create metadata for the Runtime Defined Entity: %s", err) - } - } else if _, ok := d.GetOk("metadata_entry"); ok { - return diag.Errorf("metadata_entry is only supported since VCD 10.4.0") + err = createOrUpdateOpenApiMetadataEntryInVcd(d, rde) + if err != nil { + return diag.Errorf("could not create metadata for the Runtime Defined Entity: %s", err) } return resourceVcdRdeRead(ctx, d, meta) diff --git a/vcd/resource_vcd_rde_test.go b/vcd/resource_vcd_rde_test.go index 8f83a2b25..bdca10987 100644 --- a/vcd/resource_vcd_rde_test.go +++ b/vcd/resource_vcd_rde_test.go @@ -540,10 +540,6 @@ func manipulateRde(t *testing.T, vcdClient *VCDClient, rdeId string) { // TestAccVcdRdeMetadata tests metadata CRUD on Runtime Defined Entities. func TestAccVcdRdeMetadata(t *testing.T) { skipIfNotSysAdmin(t) - vcdClient := createTemporaryVCDConnection(true) - if vcdClient != nil && vcdClient.Client.APIVCDMaxVersionIs("< 37.0") { - t.Skip("skipped as metadata for vcd_rde is only supported since VCD 10.4.0") - } testOpenApiMetadataEntryCRUD(t, testAccCheckVcdRdeMetadata, "vcd_rde.test-rde", testAccCheckVcdRdeMetadataDatasource, "data.vcd_rde.test-rde-ds", From fdf3cf466e377e3bb18898dc189042695d9c1727 Mon Sep 17 00:00:00 2001 From: abarreiro Date: Wed, 22 Nov 2023 12:57:56 +0100 Subject: [PATCH 18/39] Apply suggestion Signed-off-by: abarreiro --- vcd/resource_vcd_rde.go | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/vcd/resource_vcd_rde.go b/vcd/resource_vcd_rde.go index 569368b5d..30bef4c84 100644 --- a/vcd/resource_vcd_rde.go +++ b/vcd/resource_vcd_rde.go @@ -234,12 +234,9 @@ func resourceVcdRdeRead(_ context.Context, d *schema.ResourceData, meta interfac dSet(d, "entity_in_sync", areJsonEqual) } - // Metadata is only available since API v37.0 - if vcdClient.Client.APIVCDMaxVersionIs(">= 37.0") { - err = updateOpenApiMetadataInState(d, rde) - if err != nil { - return diag.Errorf("could not set metadata for the Runtime Defined Entity: %s", err) - } + err = updateOpenApiMetadataInState(d, rde) + if err != nil { + return diag.Errorf("could not set metadata for the Runtime Defined Entity: %s", err) } d.SetId(rde.DefinedEntity.ID) From 7e8bc3a829056b4a018525f41a3b371046770a8a Mon Sep 17 00:00:00 2001 From: abarreiro Date: Wed, 22 Nov 2023 13:12:49 +0100 Subject: [PATCH 19/39] Improve error msg Signed-off-by: abarreiro --- vcd/metadata_openapi.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/vcd/metadata_openapi.go b/vcd/metadata_openapi.go index 852fa5be2..9cc5e0b5a 100644 --- a/vcd/metadata_openapi.go +++ b/vcd/metadata_openapi.go @@ -247,7 +247,7 @@ func getOpenApiMetadataEntryMap(metadataAttribute []interface{}) (map[string]typ // In OpenAPI, metadata is namespaced, hence it is possible to have same keys but in different namespaces namespacedKey := fmt.Sprintf("%s%%%%%%%s", namespace, metadataEntry["key"].(string)) if _, ok := metadataMap[namespacedKey]; ok { - return nil, fmt.Errorf("metadata entry with %s already exists", namespacedKey) + return nil, fmt.Errorf("metadata entry with namespace '%s' and key '%s' already exists", namespace, metadataEntry["key"]) } metadataMap[namespacedKey] = types.OpenApiMetadataEntry{ From de6c0edc45845c4727c0ebadc35729f19854943b Mon Sep 17 00:00:00 2001 From: abarreiro Date: Wed, 22 Nov 2023 15:20:38 +0100 Subject: [PATCH 20/39] Fix Signed-off-by: abarreiro --- vcd/metadata_openapi.go | 5 +++++ website/docs/r/rde.html.markdown | 3 +++ 2 files changed, 8 insertions(+) diff --git a/vcd/metadata_openapi.go b/vcd/metadata_openapi.go index 9cc5e0b5a..e0e86f686 100644 --- a/vcd/metadata_openapi.go +++ b/vcd/metadata_openapi.go @@ -211,6 +211,11 @@ func getOpenApiMetadataOperations(oldMetadata []interface{}, newMetadata []inter if reflect.DeepEqual(oldEntry, newEntry) { continue } + if oldEntry.IsReadOnly != newEntry.IsReadOnly || oldEntry.IsPersistent != newEntry.IsPersistent || + oldEntry.KeyValue.Namespace != newEntry.KeyValue.Namespace || oldEntry.KeyValue.Domain != newEntry.KeyValue.Domain { + return nil, nil, nil, fmt.Errorf("only value can be updated for the entry with namespace '%s' and key '%s'. "+ + "Please delete it first and re-create if you need to change other properties", oldEntry.KeyValue.Namespace, oldEntry.KeyValue) + } metadataToUpdate[newNamespacedKey] = newEntry } } diff --git a/website/docs/r/rde.html.markdown b/website/docs/r/rde.html.markdown index 58d93165c..4d7a0e7ae 100644 --- a/website/docs/r/rde.html.markdown +++ b/website/docs/r/rde.html.markdown @@ -205,6 +205,9 @@ The `metadata_entry` is a set of metadata entries that have the following struct (e.g. Creating a copy of a VDC, capturing a vApp to a template, instantiating a catalog item as a VM...). Defaults to `false`. * `id` - (Computed) Read-only identifier for this metadata entry. +The only attribute that supports updates for a given metadata entry is `value`. If you need to modify another attribute, such as `readonly` or +`domain`, please delete and re-create the metadata entry. + Example: ```hcl From f59cf2b15e28603c62ec5d63597bb1518f3d8ecd Mon Sep 17 00:00:00 2001 From: abarreiro Date: Wed, 22 Nov 2023 15:29:52 +0100 Subject: [PATCH 21/39] # Signed-off-by: abarreiro --- vcd/metadata_openapi_common_test.go | 26 ++++++++++++++++++-------- 1 file changed, 18 insertions(+), 8 deletions(-) diff --git a/vcd/metadata_openapi_common_test.go b/vcd/metadata_openapi_common_test.go index 5ff07d915..9efdfff0a 100644 --- a/vcd/metadata_openapi_common_test.go +++ b/vcd/metadata_openapi_common_test.go @@ -7,6 +7,7 @@ import ( "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" "github.com/hashicorp/terraform-plugin-sdk/v2/terraform" "github.com/vmware/go-vcloud-director/v2/types/v56" + "regexp" "strconv" "strings" "testing" @@ -42,36 +43,41 @@ func testOpenApiMetadataEntryCRUD(t *testing.T, resourceTemplate, resourceAddres params["FuncName"] = t.Name() + "NoMetadata" params["Metadata"] = " " noMetadataHcl := templateFill(resourceTemplate, params) - debugPrintf("#[DEBUG] CONFIGURATION: %s", noMetadataHcl) + debugPrintf("#[DEBUG] CONFIGURATION NoMetadata: %s", noMetadataHcl) params["FuncName"] = t.Name() + "Create" params["Metadata"] = getOpenApiMetadataTestingHcl(1, 1, 1, 1, 2, 1, 1) createHcl := templateFill(templateWithOutput, params) - debugPrintf("#[DEBUG] CONFIGURATION: %s", createHcl) + debugPrintf("#[DEBUG] CONFIGURATION Create: %s", createHcl) params["FuncName"] = t.Name() + "WithDatasource" withDatasourceHcl := templateFill(datasourceTemplate+"\n# skip-binary-test\n"+templateWithOutput, params) - debugPrintf("#[DEBUG] CONFIGURATION: %s", withDatasourceHcl) + debugPrintf("#[DEBUG] CONFIGURATION WithDatasource: %s", withDatasourceHcl) params["FuncName"] = t.Name() + "DeleteOneKey" params["Metadata"] = getOpenApiMetadataTestingHcl(1, 1, 0, 1, 2, 1, 1) deleteOneKeyHcl := templateFill(templateWithOutput, params) - debugPrintf("#[DEBUG] CONFIGURATION: %s", deleteOneKeyHcl) + debugPrintf("#[DEBUG] CONFIGURATION DeleteOneKey: %s", deleteOneKeyHcl) params["FuncName"] = t.Name() + "Update" params["Metadata"] = strings.NewReplacer("stringValue", "stringValueUpdated").Replace(params["Metadata"].(string)) updateHcl := templateFill(templateWithOutput, params) - debugPrintf("#[DEBUG] CONFIGURATION: %s", updateHcl) + debugPrintf("#[DEBUG] CONFIGURATION Update: %s", updateHcl) + + params["FuncName"] = t.Name() + "ErrorUpdate" + params["Metadata"] = strings.NewReplacer("TENANT", "PROVIDER").Replace(params["Metadata"].(string)) + errorUpdateHcl := templateFill("# skip-binary-test\n"+templateWithOutput, params) + debugPrintf("#[DEBUG] CONFIGURATION ErrorUpdate: %s", errorUpdateHcl) params["FuncName"] = t.Name() + "Delete" params["Metadata"] = " " deleteHcl := templateFill(resourceTemplate, params) - debugPrintf("#[DEBUG] CONFIGURATION: %s", deleteHcl) + debugPrintf("#[DEBUG] CONFIGURATION Delete: %s", deleteHcl) - params["FuncName"] = t.Name() + "MetadataEntryWithDefaults" + params["FuncName"] = t.Name() + "WithDefaults" params["Metadata"] = "metadata_entry {\n\tkey = \"defaultKey\"\nvalue = \"defaultValue\"\n}" withDefaults := templateFill(resourceTemplate, params) - debugPrintf("#[DEBUG] CONFIGURATION: %s", withDefaults) + debugPrintf("#[DEBUG] CONFIGURATION WithDefaults: %s", withDefaults) if vcdShortTest { t.Skip(acceptanceTestsSkipped) @@ -178,6 +184,10 @@ func testOpenApiMetadataEntryCRUD(t *testing.T, resourceTemplate, resourceAddres testIdsDontChange(), ), }, + { + Config: errorUpdateHcl, + ExpectError: regexp.MustCompile("only value can be updated for the entry with"), + }, { Config: deleteHcl, Check: resource.ComposeAggregateTestCheckFunc( From b52704ddf82d08b614f4ca425c74346b5e8460a3 Mon Sep 17 00:00:00 2001 From: abarreiro Date: Wed, 22 Nov 2023 15:37:50 +0100 Subject: [PATCH 22/39] Change error msg Signed-off-by: abarreiro --- vcd/metadata_openapi.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/vcd/metadata_openapi.go b/vcd/metadata_openapi.go index e0e86f686..e00ab722f 100644 --- a/vcd/metadata_openapi.go +++ b/vcd/metadata_openapi.go @@ -152,7 +152,7 @@ func createOrUpdateOpenApiMetadataEntryInVcd(d *schema.ResourceData, resource op if len(r) == 2 { return r[0], r[1], nil } - return "", "", fmt.Errorf("bad formatting of metadata map key %s, this is a provider error", namespacedKey) + return "", "", fmt.Errorf("bad formatting of metadata map key %s, this is a Terraform VCD Provider error", namespacedKey) } for _, namespacedMetadataKey := range metadataToDelete { From 9211ae3ad332181fbc29665d6731b78386224077 Mon Sep 17 00:00:00 2001 From: abarreiro Date: Wed, 22 Nov 2023 15:39:35 +0100 Subject: [PATCH 23/39] Add clarification Signed-off-by: abarreiro --- vcd/metadata_openapi.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/vcd/metadata_openapi.go b/vcd/metadata_openapi.go index e00ab722f..d56185987 100644 --- a/vcd/metadata_openapi.go +++ b/vcd/metadata_openapi.go @@ -249,7 +249,9 @@ func getOpenApiMetadataEntryMap(metadataAttribute []interface{}) (map[string]typ return nil, fmt.Errorf("error parsing the 'value' attribute '%s' from state: %s", metadataEntry["value"].(string), err) } - // In OpenAPI, metadata is namespaced, hence it is possible to have same keys but in different namespaces + // In OpenAPI, metadata is namespaced, hence it is possible to have same keys but in different namespaces. + // For that reason, we use "namespace%%%key" to unequivocally identify the metadata entries (notice that %% is needed in + // Sprintf to print a single '%'). namespacedKey := fmt.Sprintf("%s%%%%%%%s", namespace, metadataEntry["key"].(string)) if _, ok := metadataMap[namespacedKey]; ok { return nil, fmt.Errorf("metadata entry with namespace '%s' and key '%s' already exists", namespace, metadataEntry["key"]) From 2fd2bb04228ce56622442e2491ee1fb088754428 Mon Sep 17 00:00:00 2001 From: abarreiro Date: Wed, 22 Nov 2023 15:40:37 +0100 Subject: [PATCH 24/39] Add clarification Signed-off-by: abarreiro --- website/docs/r/rde.html.markdown | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/website/docs/r/rde.html.markdown b/website/docs/r/rde.html.markdown index 4d7a0e7ae..db41b7169 100644 --- a/website/docs/r/rde.html.markdown +++ b/website/docs/r/rde.html.markdown @@ -225,11 +225,11 @@ resource "vcd_rde" "my-rde" { readonly = true } metadata_entry { - key = "bar" - type = "NumberEntry" - value = "42" - domain = "TENANT" - permanent = true + key = "bar" + type = "NumberEntry" + value = "42" + domain = "TENANT" + persistent = true } } ``` From d9a8b54442498e1a3993d7b8283a97a2bf6ecc43 Mon Sep 17 00:00:00 2001 From: abarreiro Date: Wed, 22 Nov 2023 15:51:54 +0100 Subject: [PATCH 25/39] Refactor and simplify code Signed-off-by: abarreiro --- vcd/metadata_openapi.go | 52 ++++++++++++++--------------------------- 1 file changed, 18 insertions(+), 34 deletions(-) diff --git a/vcd/metadata_openapi.go b/vcd/metadata_openapi.go index d56185987..c29bdaeaa 100644 --- a/vcd/metadata_openapi.go +++ b/vcd/metadata_openapi.go @@ -7,7 +7,6 @@ import ( "github.com/vmware/go-vcloud-director/v2/types/v56" "reflect" "strconv" - "strings" ) // openApiMetadataEntryDatasourceSchema returns the schema associated to the OpenAPI metadata_entry for a given data source. @@ -145,35 +144,17 @@ func createOrUpdateOpenApiMetadataEntryInVcd(d *schema.ResourceData, resource op return fmt.Errorf("could not calculate the needed metadata operations: %s", err) } - // getOpenApiMetadataOperations retrieves keys and namespaces merged with a separator, this function - // splits the values in two: namespace and key, separately. - getKeyAndNamespace := func(namespacedKey string) (string, string, error) { - r := strings.Split(namespacedKey, "%%%") // Separator used by getOpenApiMetadataEntryMap - if len(r) == 2 { - return r[0], r[1], nil - } - return "", "", fmt.Errorf("bad formatting of metadata map key %s, this is a Terraform VCD Provider error", namespacedKey) - } - - for _, namespacedMetadataKey := range metadataToDelete { - namespace, key, err := getKeyAndNamespace(namespacedMetadataKey) - if err != nil { - return err - } - err = resource.DeleteMetadata(namespace, key) + for _, entry := range metadataToDelete { + err = resource.DeleteMetadata(entry.KeyValue.Namespace, entry.KeyValue.Key) if err != nil { - return fmt.Errorf("error deleting metadata with namespace '%s' and key '%s': %s", namespace, key, err) + return fmt.Errorf("error deleting metadata with namespace '%s' and key '%s': %s", entry.KeyValue.Namespace, entry.KeyValue.Key, err) } } - for namespacedMetadataKey, metadataEntry := range metadataToUpdate { - namespace, key, err := getKeyAndNamespace(namespacedMetadataKey) - if err != nil { - return err - } - _, err = resource.UpdateMetadata(namespace, key, metadataEntry.KeyValue.Value.Value) + for _, entry := range metadataToUpdate { + _, err = resource.UpdateMetadata(entry.KeyValue.Namespace, entry.KeyValue.Key, entry.KeyValue.Value.Value) if err != nil { - return fmt.Errorf("error updating metadata with namespace '%s' and key '%s': %s", namespace, key, err) + return fmt.Errorf("error updating metadata with namespace '%s' and key '%s': %s", entry.KeyValue.Namespace, entry.KeyValue.Key, err) } } @@ -188,7 +169,7 @@ func createOrUpdateOpenApiMetadataEntryInVcd(d *schema.ResourceData, resource op // getOpenApiMetadataOperations retrieves the metadata that needs to be added, to be updated and to be deleted depending // on the old and new attribute values from Terraform state. -func getOpenApiMetadataOperations(oldMetadata []interface{}, newMetadata []interface{}) ([]types.OpenApiMetadataEntry, map[string]types.OpenApiMetadataEntry, []string, error) { +func getOpenApiMetadataOperations(oldMetadata []interface{}, newMetadata []interface{}) ([]types.OpenApiMetadataEntry, []types.OpenApiMetadataEntry, []types.OpenApiMetadataEntry, error) { oldMetadataEntries, err := getOpenApiMetadataEntryMap(oldMetadata) if err != nil { return nil, nil, nil, err @@ -198,14 +179,14 @@ func getOpenApiMetadataOperations(oldMetadata []interface{}, newMetadata []inter return nil, nil, nil, err } - var metadataToRemove []string + var metadataToRemove []types.OpenApiMetadataEntry for oldNamespacedKey := range oldMetadataEntries { if _, ok := newMetadataEntries[oldNamespacedKey]; !ok { - metadataToRemove = append(metadataToRemove, oldNamespacedKey) + metadataToRemove = append(metadataToRemove, oldMetadataEntries[oldNamespacedKey]) } } - metadataToUpdate := map[string]types.OpenApiMetadataEntry{} + metadataToUpdateMap := map[string]types.OpenApiMetadataEntry{} for newNamespacedKey, newEntry := range newMetadataEntries { if oldEntry, ok := oldMetadataEntries[newNamespacedKey]; ok { if reflect.DeepEqual(oldEntry, newEntry) { @@ -216,14 +197,18 @@ func getOpenApiMetadataOperations(oldMetadata []interface{}, newMetadata []inter return nil, nil, nil, fmt.Errorf("only value can be updated for the entry with namespace '%s' and key '%s'. "+ "Please delete it first and re-create if you need to change other properties", oldEntry.KeyValue.Namespace, oldEntry.KeyValue) } - metadataToUpdate[newNamespacedKey] = newEntry + metadataToUpdateMap[newNamespacedKey] = newEntry } } + var metadataToUpdate []types.OpenApiMetadataEntry + for _, v := range metadataToUpdateMap { + metadataToUpdate = append(metadataToUpdate, v) + } var metadataToCreate []types.OpenApiMetadataEntry for newNamespacedKey, newEntry := range newMetadataEntries { _, alreadyExisting := oldMetadataEntries[newNamespacedKey] - _, beingUpdated := metadataToUpdate[newNamespacedKey] + _, beingUpdated := metadataToUpdateMap[newNamespacedKey] if !alreadyExisting && !beingUpdated { metadataToCreate = append(metadataToCreate, newEntry) } @@ -250,9 +235,8 @@ func getOpenApiMetadataEntryMap(metadataAttribute []interface{}) (map[string]typ } // In OpenAPI, metadata is namespaced, hence it is possible to have same keys but in different namespaces. - // For that reason, we use "namespace%%%key" to unequivocally identify the metadata entries (notice that %% is needed in - // Sprintf to print a single '%'). - namespacedKey := fmt.Sprintf("%s%%%%%%%s", namespace, metadataEntry["key"].(string)) + // For that reason, we use "namespace+key" to unequivocally identify the metadata entries. + namespacedKey := fmt.Sprintf("%s%s", namespace, metadataEntry["key"].(string)) if _, ok := metadataMap[namespacedKey]; ok { return nil, fmt.Errorf("metadata entry with namespace '%s' and key '%s' already exists", namespace, metadataEntry["key"]) } From 67af8a5b020402e4867b98b704dbcd899633a072 Mon Sep 17 00:00:00 2001 From: abarreiro Date: Wed, 22 Nov 2023 17:24:13 +0100 Subject: [PATCH 26/39] Add test Signed-off-by: abarreiro --- vcd/resource_vcd_rde_test.go | 141 +++++++++++++++++++++++++++++++++-- 1 file changed, 135 insertions(+), 6 deletions(-) diff --git a/vcd/resource_vcd_rde_test.go b/vcd/resource_vcd_rde_test.go index bdca10987..45dc20879 100644 --- a/vcd/resource_vcd_rde_test.go +++ b/vcd/resource_vcd_rde_test.go @@ -541,8 +541,8 @@ func manipulateRde(t *testing.T, vcdClient *VCDClient, rdeId string) { func TestAccVcdRdeMetadata(t *testing.T) { skipIfNotSysAdmin(t) testOpenApiMetadataEntryCRUD(t, - testAccCheckVcdRdeMetadata, "vcd_rde.test-rde", - testAccCheckVcdRdeMetadataDatasource, "data.vcd_rde.test-rde-ds", + testAccCheckVcdRdeMetadata, "vcd_rde.test_rde", + testAccCheckVcdRdeMetadataDatasource, "data.vcd_rde.test_rde_ds", StringMap{}) } @@ -552,7 +552,7 @@ data "vcd_rde_type" "rde_type" { nss = "tkgcluster" version = "1.0.0" } -resource "vcd_rde" "test-rde" { +resource "vcd_rde" "test_rde" { org = "System" rde_type_id = data.vcd_rde_type.rde_type.id name = "{{.Name}}" @@ -563,9 +563,138 @@ resource "vcd_rde" "test-rde" { ` const testAccCheckVcdRdeMetadataDatasource = ` -data "vcd_rde" "test-rde-ds" { +data "vcd_rde" "test_rde_ds" { org = "System" - rde_type_id = vcd_rde.test-rde.rde_type_id - name = vcd_rde.test-rde.name + rde_type_id = vcd_rde.test_rde.rde_type_id + name = vcd_rde.test_rde.name +} +` + +// TestAccVcdRdeTenantMetadata tests that a tenant user cannot read metadata that was created in the RDE with domain = "PROVIDER". +// It will only be able to read those entries with domain = "TENANT". +func TestAccVcdRdeTenantMetadata(t *testing.T) { + skipIfNotSysAdmin(t) + preTestChecks(t) + var params = StringMap{ + "FuncName": t.Name() + "-Step1", + "ProviderVcdOrg1": providerVcdOrg1, + "Org": testConfig.VCD.Org, + "Name": t.Name(), + "Metadata": getOpenApiMetadataTestingHcl(1, 1, 1, 1, 2, 1, 1), + "SchemaPath": getCurrentDir() + "/../test-resources/rde_type.json", + } + testParamsNotEmpty(t, params) + + step1 := templateFill(testAccCheckVcdRdeTenantMetadata, params) + debugPrintf("#[DEBUG] CONFIGURATION Step 1: %s", step1) + + params["FuncName"] = t.Name() + "-Step2" + step2 := templateFill(testAccCheckVcdRdeTenantMetadata+testAccCheckVcdRdeMetadataTenantDatasource, params) + debugPrintf("#[DEBUG] CONFIGURATION Step 2: %s", step2) + + if vcdShortTest { + t.Skip(acceptanceTestsSkipped) + return + } + resourceName := "vcd_rde.test_rde" + datasourceName := "data.vcd_rde.test_rde_ds" + + // Required to manipulate rights + vcdClient := createTemporaryVCDConnection(true) + if vcdClient == nil || vcdClient.VCDClient == nil { + t.Errorf("could not get a VCD connection to add rights to tenant user") + } + + resource.Test(t, resource.TestCase{ + ProviderFactories: buildMultipleProviders(), + Steps: []resource.TestStep{ + { + Config: step1, + Check: resource.ComposeAggregateTestCheckFunc( + resource.TestCheckResourceAttr(resourceName, "name", t.Name()), + resource.TestCheckResourceAttr(resourceName, "metadata_entry.#", "8"), + testCheckOpenApiMetadataEntrySetElemNestedAttrs(resourceName, "stringKey1", "stringValue1", types.OpenApiMetadataStringEntry, "TENANT", "", "false", "false"), + testCheckOpenApiMetadataEntrySetElemNestedAttrs(resourceName, "numberKey1", "1", types.OpenApiMetadataNumberEntry, "TENANT", "", "false", "false"), + testCheckOpenApiMetadataEntrySetElemNestedAttrs(resourceName, "boolKey1", "false", types.OpenApiMetadataStringEntry, "TENANT", "", "false", "false"), + testCheckOpenApiMetadataEntrySetElemNestedAttrs(resourceName, "readOnly1", "readOnly1", types.OpenApiMetadataStringEntry, "TENANT", "", "true", "false"), + testCheckOpenApiMetadataEntrySetElemNestedAttrs(resourceName, "namespace", "namespace1", types.OpenApiMetadataStringEntry, "TENANT", "namespace1", "false", "false"), + testCheckOpenApiMetadataEntrySetElemNestedAttrs(resourceName, "namespace", "namespace2", types.OpenApiMetadataStringEntry, "TENANT", "namespace2", "false", "false"), + testCheckOpenApiMetadataEntrySetElemNestedAttrs(resourceName, "provider1", "provider1", types.OpenApiMetadataStringEntry, "PROVIDER", "", "false", "false"), + testCheckOpenApiMetadataEntrySetElemNestedAttrs(resourceName, "persistent1", "persistent1", types.OpenApiMetadataStringEntry, "TENANT", "", "false", "true"), + ), + }, + { + Config: step2, + PreConfig: func() { + // Rights will be deleted with the destruction of the RDE Type. + addRightsToTenantUser(t, vcdClient, "vmware", params["Name"].(string)) + // We need to invalidate existing client cache and start a new one as the rights for the tenant user have changed, hence + // we can't reuse existing sessions + cachedVCDClients.reset() + }, + Check: resource.ComposeAggregateTestCheckFunc( + resource.TestCheckResourceAttr(datasourceName, "name", t.Name()), + resource.TestCheckResourceAttr(datasourceName, "metadata_entry.#", "7"), + testCheckOpenApiMetadataEntrySetElemNestedAttrs(resourceName, "stringKey1", "stringValue1", types.OpenApiMetadataStringEntry, "TENANT", "", "false", "false"), + testCheckOpenApiMetadataEntrySetElemNestedAttrs(resourceName, "numberKey1", "1", types.OpenApiMetadataNumberEntry, "TENANT", "", "false", "false"), + testCheckOpenApiMetadataEntrySetElemNestedAttrs(resourceName, "boolKey1", "false", types.OpenApiMetadataStringEntry, "TENANT", "", "false", "false"), + testCheckOpenApiMetadataEntrySetElemNestedAttrs(resourceName, "readOnly1", "readOnly1", types.OpenApiMetadataStringEntry, "TENANT", "", "true", "false"), + testCheckOpenApiMetadataEntrySetElemNestedAttrs(resourceName, "namespace", "namespace1", types.OpenApiMetadataStringEntry, "TENANT", "namespace1", "false", "false"), + testCheckOpenApiMetadataEntrySetElemNestedAttrs(resourceName, "namespace", "namespace2", types.OpenApiMetadataStringEntry, "TENANT", "namespace2", "false", "false"), + testCheckOpenApiMetadataEntrySetElemNestedAttrs(resourceName, "persistent1", "persistent1", types.OpenApiMetadataStringEntry, "TENANT", "", "false", "true"), + ), + }, + }, + }) + postTestChecks(t) +} + +const testAccCheckVcdRdeTenantMetadata = ` +# skip-binary: This is already tested by TestAccVcdRdeMetadata +resource "vcd_rde_type" "rde_type" { + name = "{{.Name}}" + vendor = "vmware" + nss = "{{.Name}}" + version = "1.0.0" + schema = file("{{.SchemaPath}}") +} + +# This is required because the organization where the RDE is created +# is lacking of the rights created by the type above. +# This bundle will be automatically removed by VCD with the destruction of the type. +resource "vcd_rights_bundle" "rde_type_bundle" { + name = "{{.Name}} bundle" + description = "{{.Name}} bundle" + publish_to_all_tenants = true + rights = [ + "vmware:{{.Name}}: Administrator Full access", + "vmware:{{.Name}}: Full Access", + "vmware:{{.Name}}: Modify", + "vmware:{{.Name}}: View", + "vmware:{{.Name}}: Administrator View", + ] + depends_on = [vcd_rde_type.rde_type] +} + +resource "vcd_rde" "test_rde" { + org = "{{.Org}}" + rde_type_id = vcd_rde_type.rde_type.id + name = "{{.Name}}" + input_entity = "{\"foo\":\"bar\"}" # We are just testing metadata so we don't care about entity state + resolve = true + {{.Metadata}} + + depends_on = [vcd_rights_bundle.rde_type_bundle] # We need to wait for the rights to be published, otherwise creation will fail +} +` + +const testAccCheckVcdRdeMetadataTenantDatasource = ` +# skip-binary: This requires manual publishing of rights +data "vcd_rde" "test_rde_ds" { + provider = {{.ProviderVcdOrg1}} + + org = vcd_rde.test_rde.org + rde_type_id = vcd_rde.test_rde.rde_type_id + name = vcd_rde.test_rde.name } ` From 3eab5aeb6edb56228fa9aa97221a5daaefa505ae Mon Sep 17 00:00:00 2001 From: abarreiro Date: Wed, 22 Nov 2023 17:27:34 +0100 Subject: [PATCH 27/39] Simplify test Signed-off-by: abarreiro --- vcd/resource_vcd_rde_test.go | 20 +++++--------------- 1 file changed, 5 insertions(+), 15 deletions(-) diff --git a/vcd/resource_vcd_rde_test.go b/vcd/resource_vcd_rde_test.go index 45dc20879..2491c2879 100644 --- a/vcd/resource_vcd_rde_test.go +++ b/vcd/resource_vcd_rde_test.go @@ -580,7 +580,7 @@ func TestAccVcdRdeTenantMetadata(t *testing.T) { "ProviderVcdOrg1": providerVcdOrg1, "Org": testConfig.VCD.Org, "Name": t.Name(), - "Metadata": getOpenApiMetadataTestingHcl(1, 1, 1, 1, 2, 1, 1), + "Metadata": getOpenApiMetadataTestingHcl(1, 0, 0, 0, 0, 3, 0), "SchemaPath": getCurrentDir() + "/../test-resources/rde_type.json", } testParamsNotEmpty(t, params) @@ -612,15 +612,11 @@ func TestAccVcdRdeTenantMetadata(t *testing.T) { Config: step1, Check: resource.ComposeAggregateTestCheckFunc( resource.TestCheckResourceAttr(resourceName, "name", t.Name()), - resource.TestCheckResourceAttr(resourceName, "metadata_entry.#", "8"), + resource.TestCheckResourceAttr(resourceName, "metadata_entry.#", "4"), testCheckOpenApiMetadataEntrySetElemNestedAttrs(resourceName, "stringKey1", "stringValue1", types.OpenApiMetadataStringEntry, "TENANT", "", "false", "false"), - testCheckOpenApiMetadataEntrySetElemNestedAttrs(resourceName, "numberKey1", "1", types.OpenApiMetadataNumberEntry, "TENANT", "", "false", "false"), - testCheckOpenApiMetadataEntrySetElemNestedAttrs(resourceName, "boolKey1", "false", types.OpenApiMetadataStringEntry, "TENANT", "", "false", "false"), - testCheckOpenApiMetadataEntrySetElemNestedAttrs(resourceName, "readOnly1", "readOnly1", types.OpenApiMetadataStringEntry, "TENANT", "", "true", "false"), - testCheckOpenApiMetadataEntrySetElemNestedAttrs(resourceName, "namespace", "namespace1", types.OpenApiMetadataStringEntry, "TENANT", "namespace1", "false", "false"), - testCheckOpenApiMetadataEntrySetElemNestedAttrs(resourceName, "namespace", "namespace2", types.OpenApiMetadataStringEntry, "TENANT", "namespace2", "false", "false"), testCheckOpenApiMetadataEntrySetElemNestedAttrs(resourceName, "provider1", "provider1", types.OpenApiMetadataStringEntry, "PROVIDER", "", "false", "false"), - testCheckOpenApiMetadataEntrySetElemNestedAttrs(resourceName, "persistent1", "persistent1", types.OpenApiMetadataStringEntry, "TENANT", "", "false", "true"), + testCheckOpenApiMetadataEntrySetElemNestedAttrs(resourceName, "provider2", "provider2", types.OpenApiMetadataStringEntry, "PROVIDER", "", "false", "false"), + testCheckOpenApiMetadataEntrySetElemNestedAttrs(resourceName, "provider3", "provider3", types.OpenApiMetadataStringEntry, "PROVIDER", "", "false", "false"), ), }, { @@ -634,14 +630,8 @@ func TestAccVcdRdeTenantMetadata(t *testing.T) { }, Check: resource.ComposeAggregateTestCheckFunc( resource.TestCheckResourceAttr(datasourceName, "name", t.Name()), - resource.TestCheckResourceAttr(datasourceName, "metadata_entry.#", "7"), + resource.TestCheckResourceAttr(datasourceName, "metadata_entry.#", "1"), testCheckOpenApiMetadataEntrySetElemNestedAttrs(resourceName, "stringKey1", "stringValue1", types.OpenApiMetadataStringEntry, "TENANT", "", "false", "false"), - testCheckOpenApiMetadataEntrySetElemNestedAttrs(resourceName, "numberKey1", "1", types.OpenApiMetadataNumberEntry, "TENANT", "", "false", "false"), - testCheckOpenApiMetadataEntrySetElemNestedAttrs(resourceName, "boolKey1", "false", types.OpenApiMetadataStringEntry, "TENANT", "", "false", "false"), - testCheckOpenApiMetadataEntrySetElemNestedAttrs(resourceName, "readOnly1", "readOnly1", types.OpenApiMetadataStringEntry, "TENANT", "", "true", "false"), - testCheckOpenApiMetadataEntrySetElemNestedAttrs(resourceName, "namespace", "namespace1", types.OpenApiMetadataStringEntry, "TENANT", "namespace1", "false", "false"), - testCheckOpenApiMetadataEntrySetElemNestedAttrs(resourceName, "namespace", "namespace2", types.OpenApiMetadataStringEntry, "TENANT", "namespace2", "false", "false"), - testCheckOpenApiMetadataEntrySetElemNestedAttrs(resourceName, "persistent1", "persistent1", types.OpenApiMetadataStringEntry, "TENANT", "", "false", "true"), ), }, }, From f75423ea0c70e72165e6ebc209f680a086b3c45e Mon Sep 17 00:00:00 2001 From: abarreiro Date: Thu, 23 Nov 2023 09:30:58 +0100 Subject: [PATCH 28/39] Add ignore_metadata_changes note Signed-off-by: abarreiro --- website/docs/r/rde.html.markdown | 2 ++ 1 file changed, 2 insertions(+) diff --git a/website/docs/r/rde.html.markdown b/website/docs/r/rde.html.markdown index db41b7169..22ee914af 100644 --- a/website/docs/r/rde.html.markdown +++ b/website/docs/r/rde.html.markdown @@ -193,6 +193,8 @@ resolved by anyone. a change in of one or more of them, but applying it will respect every non-changed metadata entry by not performing any modification on these. +~> The provider configuration block `ignore_metadata_changes` is not compatible with metadata entries of `vcd_rde`. + The `metadata_entry` is a set of metadata entries that have the following structure: * `key` - (Required) Key of this metadata entry. From 56414e490bc0f074f7556bfa85619f2e350045c1 Mon Sep 17 00:00:00 2001 From: abarreiro Date: Thu, 23 Nov 2023 12:52:30 +0100 Subject: [PATCH 29/39] Apply suggestion Signed-off-by: abarreiro --- website/docs/r/rde.html.markdown | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/website/docs/r/rde.html.markdown b/website/docs/r/rde.html.markdown index 22ee914af..6b31dd23b 100644 --- a/website/docs/r/rde.html.markdown +++ b/website/docs/r/rde.html.markdown @@ -189,9 +189,9 @@ resolved by anyone. ## Metadata --> Due to Terraform limitations, `terraform plan` shows that all metadata entries will be deleted and re-created when performing -a change in of one or more of them, but applying it will respect every non-changed metadata entry by not performing any modification on -these. +-> Due to Terraform limitations, when performing a change in any of the metadata entries, a `terraform plan` shows that +all of them will be deleted and re-created. However, the succeeding `terraform apply` will respect the non-changed metadata entries +by not modifying them. ~> The provider configuration block `ignore_metadata_changes` is not compatible with metadata entries of `vcd_rde`. From 8db90b96f4a94836fdb825f5af2a232b056034c4 Mon Sep 17 00:00:00 2001 From: abarreiro Date: Thu, 23 Nov 2023 13:41:56 +0100 Subject: [PATCH 30/39] gs Signed-off-by: abarreiro --- vcd/metadata_openapi.go | 15 ++++++++++----- vcd/metadata_openapi_common_test.go | 25 ++++++++++++++++++------- 2 files changed, 28 insertions(+), 12 deletions(-) diff --git a/vcd/metadata_openapi.go b/vcd/metadata_openapi.go index c29bdaeaa..3cce11e34 100644 --- a/vcd/metadata_openapi.go +++ b/vcd/metadata_openapi.go @@ -5,6 +5,7 @@ import ( "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/validation" "github.com/vmware/go-vcloud-director/v2/types/v56" + "github.com/vmware/go-vcloud-director/v2/util" "reflect" "strconv" ) @@ -186,6 +187,7 @@ func getOpenApiMetadataOperations(oldMetadata []interface{}, newMetadata []inter } } + var metadataToCreate []types.OpenApiMetadataEntry metadataToUpdateMap := map[string]types.OpenApiMetadataEntry{} for newNamespacedKey, newEntry := range newMetadataEntries { if oldEntry, ok := oldMetadataEntries[newNamespacedKey]; ok { @@ -193,11 +195,15 @@ func getOpenApiMetadataOperations(oldMetadata []interface{}, newMetadata []inter continue } if oldEntry.IsReadOnly != newEntry.IsReadOnly || oldEntry.IsPersistent != newEntry.IsPersistent || - oldEntry.KeyValue.Namespace != newEntry.KeyValue.Namespace || oldEntry.KeyValue.Domain != newEntry.KeyValue.Domain { - return nil, nil, nil, fmt.Errorf("only value can be updated for the entry with namespace '%s' and key '%s'. "+ - "Please delete it first and re-create if you need to change other properties", oldEntry.KeyValue.Namespace, oldEntry.KeyValue) + oldEntry.KeyValue.Namespace != newEntry.KeyValue.Namespace || oldEntry.KeyValue.Domain != newEntry.KeyValue.Domain || + oldEntry.KeyValue.Value.Type != newEntry.KeyValue.Value.Type { + util.Logger.Printf("[DEBUG] entry with namespace '%s' and key '%s' is being deleted and re-created", oldEntry.KeyValue.Namespace, oldEntry.KeyValue.Key) + metadataToRemove = append(metadataToRemove, oldMetadataEntries[newNamespacedKey]) + metadataToCreate = append(metadataToCreate, newMetadataEntries[newNamespacedKey]) + } else { + metadataToUpdateMap[newNamespacedKey] = newEntry } - metadataToUpdateMap[newNamespacedKey] = newEntry + } } var metadataToUpdate []types.OpenApiMetadataEntry @@ -205,7 +211,6 @@ func getOpenApiMetadataOperations(oldMetadata []interface{}, newMetadata []inter metadataToUpdate = append(metadataToUpdate, v) } - var metadataToCreate []types.OpenApiMetadataEntry for newNamespacedKey, newEntry := range newMetadataEntries { _, alreadyExisting := oldMetadataEntries[newNamespacedKey] _, beingUpdated := metadataToUpdateMap[newNamespacedKey] diff --git a/vcd/metadata_openapi_common_test.go b/vcd/metadata_openapi_common_test.go index 9efdfff0a..b6b96e545 100644 --- a/vcd/metadata_openapi_common_test.go +++ b/vcd/metadata_openapi_common_test.go @@ -7,7 +7,6 @@ import ( "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" "github.com/hashicorp/terraform-plugin-sdk/v2/terraform" "github.com/vmware/go-vcloud-director/v2/types/v56" - "regexp" "strconv" "strings" "testing" @@ -64,10 +63,10 @@ func testOpenApiMetadataEntryCRUD(t *testing.T, resourceTemplate, resourceAddres updateHcl := templateFill(templateWithOutput, params) debugPrintf("#[DEBUG] CONFIGURATION Update: %s", updateHcl) - params["FuncName"] = t.Name() + "ErrorUpdate" - params["Metadata"] = strings.NewReplacer("TENANT", "PROVIDER").Replace(params["Metadata"].(string)) - errorUpdateHcl := templateFill("# skip-binary-test\n"+templateWithOutput, params) - debugPrintf("#[DEBUG] CONFIGURATION ErrorUpdate: %s", errorUpdateHcl) + params["FuncName"] = t.Name() + "UpdateForceRecreate" + params["Metadata"] = strings.NewReplacer("NumberEntry", "StringEntry").Replace(params["Metadata"].(string)) + updateForceRecreateHcl := templateFill(templateWithOutput, params) + debugPrintf("#[DEBUG] CONFIGURATION UpdateForceRecreate: %s", updateForceRecreateHcl) params["FuncName"] = t.Name() + "Delete" params["Metadata"] = " " @@ -185,8 +184,20 @@ func testOpenApiMetadataEntryCRUD(t *testing.T, resourceTemplate, resourceAddres ), }, { - Config: errorUpdateHcl, - ExpectError: regexp.MustCompile("only value can be updated for the entry with"), + Config: updateForceRecreateHcl, + Check: resource.ComposeAggregateTestCheckFunc( + resource.TestCheckResourceAttr(resourceAddress, "name", t.Name()), + resource.TestCheckResourceAttr(resourceAddress, "metadata_entry.#", "7"), + // Updated value, from number to string, should force a recreation of the entry: + testCheckOpenApiMetadataEntrySetElemNestedAttrs(resourceAddress, "numberKey1", "1", types.OpenApiMetadataStringEntry, "TENANT", "", "false", "false"), + // Not updated values: + testCheckOpenApiMetadataEntrySetElemNestedAttrs(resourceAddress, "stringKey1", "stringValueUpdated1", types.OpenApiMetadataStringEntry, "TENANT", "", "false", "false"), + testCheckOpenApiMetadataEntrySetElemNestedAttrs(resourceAddress, "readOnly1", "readOnly1", types.OpenApiMetadataStringEntry, "TENANT", "", "true", "false"), + testCheckOpenApiMetadataEntrySetElemNestedAttrs(resourceAddress, "namespace", "namespace1", types.OpenApiMetadataStringEntry, "TENANT", "namespace1", "false", "false"), + testCheckOpenApiMetadataEntrySetElemNestedAttrs(resourceAddress, "namespace", "namespace2", types.OpenApiMetadataStringEntry, "TENANT", "namespace2", "false", "false"), + testCheckOpenApiMetadataEntrySetElemNestedAttrs(resourceAddress, "provider1", "provider1", types.OpenApiMetadataStringEntry, "PROVIDER", "", "false", "false"), + testCheckOpenApiMetadataEntrySetElemNestedAttrs(resourceAddress, "persistent1", "persistent1", types.OpenApiMetadataStringEntry, "TENANT", "", "false", "true"), + ), }, { Config: deleteHcl, From 2fa0e021118e402c5e8f32938e429198ec587303 Mon Sep 17 00:00:00 2001 From: abarreiro Date: Thu, 23 Nov 2023 14:34:42 +0100 Subject: [PATCH 31/39] Update docs Signed-off-by: abarreiro --- website/docs/r/rde.html.markdown | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/website/docs/r/rde.html.markdown b/website/docs/r/rde.html.markdown index 6b31dd23b..ec7b4088f 100644 --- a/website/docs/r/rde.html.markdown +++ b/website/docs/r/rde.html.markdown @@ -198,13 +198,16 @@ by not modifying them. The `metadata_entry` is a set of metadata entries that have the following structure: * `key` - (Required) Key of this metadata entry. -* `value` - (Required) Value of this metadata entry. -* `type` - (Optional) Type of this metadata entry. One of: `StringEntry`, `NumberEntry`, `BoolEntry`. Defaults to `StringEntry`. * `namespace` - (Optional) Namespace of the metadata entry. Allows having multiple entries with same key in different namespaces. +* `value` - (Required) Value of this metadata entry. It can be updated. +* `type` - (Optional) Type of this metadata entry. One of: `StringEntry`, `NumberEntry`, `BoolEntry`. Defaults to `StringEntry`. + Updating this value forces a re-creation of the metadata entry. * `domain` - (Optional) Only meaningful for providers. Allows them to share entries with their tenants. Currently, accepted values are: `TENANT`, `PROVIDER`. Defaults to `TENANT`. -* `readonly` - (Optional) `true` if the metadata entry is read only. Defaults to `false`. + Updating this value forces a re-creation of the metadata entry. +* `readonly` - (Optional) `true` if the metadata entry is read only. Defaults to `false`. Updating this value forces a re-creation of the metadata entry. * `persistent` - (Optional) `true` if the metadata is persistent. Persistent entries can be copied over on some entity operation (e.g. Creating a copy of a VDC, capturing a vApp to a template, instantiating a catalog item as a VM...). Defaults to `false`. + Updating this value forces a re-creation of the metadata entry. * `id` - (Computed) Read-only identifier for this metadata entry. The only attribute that supports updates for a given metadata entry is `value`. If you need to modify another attribute, such as `readonly` or From 6b946e8a67ea4a07c45ea212bf056ff200463a0d Mon Sep 17 00:00:00 2001 From: abarreiro Date: Thu, 23 Nov 2023 14:36:09 +0100 Subject: [PATCH 32/39] Update docs Signed-off-by: abarreiro --- website/docs/r/rde.html.markdown | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/website/docs/r/rde.html.markdown b/website/docs/r/rde.html.markdown index ec7b4088f..3f9044c13 100644 --- a/website/docs/r/rde.html.markdown +++ b/website/docs/r/rde.html.markdown @@ -210,8 +210,8 @@ The `metadata_entry` is a set of metadata entries that have the following struct Updating this value forces a re-creation of the metadata entry. * `id` - (Computed) Read-only identifier for this metadata entry. -The only attribute that supports updates for a given metadata entry is `value`. If you need to modify another attribute, such as `readonly` or -`domain`, please delete and re-create the metadata entry. +The only attribute that supports updates-in-place for a given metadata entry is `value`. Updating any other value will re-create the +metadata entry. Example: From 8c78aba5b4343a06f2750ef8b11e10d1e6cd6ebd Mon Sep 17 00:00:00 2001 From: abarreiro Date: Thu, 23 Nov 2023 17:34:52 +0100 Subject: [PATCH 33/39] Apply suggestions Signed-off-by: abarreiro --- vcd/metadata_common_test.go | 14 +++++++------- vcd/metadata_openapi.go | 2 ++ vcd/metadata_openapi_common_test.go | 28 ++++++++++++++-------------- website/docs/r/rde.html.markdown | 4 ++-- 4 files changed, 25 insertions(+), 23 deletions(-) diff --git a/vcd/metadata_common_test.go b/vcd/metadata_common_test.go index 477cfbd7f..c95c37f7e 100644 --- a/vcd/metadata_common_test.go +++ b/vcd/metadata_common_test.go @@ -608,13 +608,13 @@ func getMetadataEntryHcl(key, value, typedValue, userAccess, isSystem string) st isSystemAttr = fmt.Sprintf("is_system = \"%s\"", isSystem) } return fmt.Sprintf(` -metadata_entry { - %s - %s - %s - %s - %s -} + metadata_entry { + %s + %s + %s + %s + %s + } `, keyAttr, valueAttr, typeAttr, userAccAttr, isSystemAttr) } diff --git a/vcd/metadata_openapi.go b/vcd/metadata_openapi.go index 3cce11e34..8b0c71ba4 100644 --- a/vcd/metadata_openapi.go +++ b/vcd/metadata_openapi.go @@ -194,6 +194,7 @@ func getOpenApiMetadataOperations(oldMetadata []interface{}, newMetadata []inter if reflect.DeepEqual(oldEntry, newEntry) { continue } + // If a metadata property that is not "Value" is changed, it needs to be recreated if oldEntry.IsReadOnly != newEntry.IsReadOnly || oldEntry.IsPersistent != newEntry.IsPersistent || oldEntry.KeyValue.Namespace != newEntry.KeyValue.Namespace || oldEntry.KeyValue.Domain != newEntry.KeyValue.Domain || oldEntry.KeyValue.Value.Type != newEntry.KeyValue.Value.Type { @@ -201,6 +202,7 @@ func getOpenApiMetadataOperations(oldMetadata []interface{}, newMetadata []inter metadataToRemove = append(metadataToRemove, oldMetadataEntries[newNamespacedKey]) metadataToCreate = append(metadataToCreate, newMetadataEntries[newNamespacedKey]) } else { + // Only "Value" is changed, it can be updated metadataToUpdateMap[newNamespacedKey] = newEntry } diff --git a/vcd/metadata_openapi_common_test.go b/vcd/metadata_openapi_common_test.go index b6b96e545..e031b195c 100644 --- a/vcd/metadata_openapi_common_test.go +++ b/vcd/metadata_openapi_common_test.go @@ -28,10 +28,10 @@ func testOpenApiMetadataEntryCRUD(t *testing.T, resourceTemplate, resourceAddres // This output allows to perform some assertions on the ID inside a TypeSet, // which is impossible to obtain otherwise. outputHcl := ` - output "metadata_id" { - value = tolist(` + resourceAddress + `.metadata_entry)[0].id - } - ` +output "metadata_id" { + value = tolist(` + resourceAddress + `.metadata_entry)[0].id +} +` templateWithOutput := resourceTemplate + outputHcl for extraParam, extraParamValue := range extraParams { @@ -251,19 +251,19 @@ func getOpenApiMetadataTestingHcl(stringEntries, numberEntries, boolEntries, rea func getOpenApiMetadataEntryHcl(key, value, typedValue, domain, namespace, readonly, persistent string) string { hclNamespace := "" if namespace != "" { - hclNamespace = `namespace = "` + namespace + `"` + hclNamespace = `namespace = "` + namespace + `"` } return ` - metadata_entry { - key = "` + key + `" - value = "` + value + `" - type = "` + typedValue + `" - domain = "` + domain + `" - ` + hclNamespace + ` - readonly = ` + readonly + ` - persistent = ` + persistent + ` - }` + metadata_entry { + key = "` + key + `" + value = "` + value + `" + type = "` + typedValue + `" + domain = "` + domain + `" + ` + hclNamespace + ` + readonly = ` + readonly + ` + persistent = ` + persistent + ` + }` } // testCheckOpenApiMetadataEntrySetElemNestedAttrs asserts that a given metadata_entry has the expected input for the given resourceAddress. diff --git a/website/docs/r/rde.html.markdown b/website/docs/r/rde.html.markdown index 3f9044c13..403b1ba9c 100644 --- a/website/docs/r/rde.html.markdown +++ b/website/docs/r/rde.html.markdown @@ -226,14 +226,14 @@ resource "vcd_rde" "my-rde" { key = "foo" type = "StringEntry" value = "bar" - domain = "TENANT" + domain = "TENANT" # will be also visible to an organization user readonly = true } metadata_entry { key = "bar" type = "NumberEntry" value = "42" - domain = "TENANT" + domain = "PROVIDER" # will be only visible to the provider persistent = true } } From aa9ad339a723ec40af621e6469d161887c852e9e Mon Sep 17 00:00:00 2001 From: abarreiro Date: Fri, 24 Nov 2023 14:51:25 +0100 Subject: [PATCH 34/39] Refactoring Signed-off-by: abarreiro --- go.mod | 2 +- go.sum | 4 ++-- vcd/metadata_openapi.go | 46 ++++++++++++++++++++++++----------------- 3 files changed, 30 insertions(+), 22 deletions(-) diff --git a/go.mod b/go.mod index 60d813aef..8269b621c 100644 --- a/go.mod +++ b/go.mod @@ -66,4 +66,4 @@ require ( google.golang.org/protobuf v1.31.0 // indirect ) -replace github.com/vmware/go-vcloud-director/v2 => github.com/adambarreiro/go-vcloud-director/v2 v2.17.0-alpha.1.0.20231122110403-a2835e0d3940 +replace github.com/vmware/go-vcloud-director/v2 => github.com/adambarreiro/go-vcloud-director/v2 v2.17.0-alpha.1.0.20231124133658-498565f7a0cc diff --git a/go.sum b/go.sum index 627ee509c..0769f82e3 100644 --- a/go.sum +++ b/go.sum @@ -3,8 +3,8 @@ github.com/Microsoft/go-winio v0.6.1 h1:9/kr64B9VUZrLm5YYwbGtUJnMgqWVOdUAXu6Migc github.com/ProtonMail/go-crypto v0.0.0-20230717121422-5aa5874ade95 h1:KLq8BE0KwCL+mmXnjLWEAOYO+2l2AE4YMmqG1ZpZHBs= github.com/ProtonMail/go-crypto v0.0.0-20230717121422-5aa5874ade95/go.mod h1:EjAoLdwvbIOoOQr3ihjnSoLZRtE8azugULFRteWMNc0= github.com/acomagu/bufpipe v1.0.4 h1:e3H4WUzM3npvo5uv95QuJM3cQspFNtFBzvJ2oNjKIDQ= -github.com/adambarreiro/go-vcloud-director/v2 v2.17.0-alpha.1.0.20231122110403-a2835e0d3940 h1:3r8zHFAK+p+gTkcYWqlm59MSEwFZTk307snLYFbxLDw= -github.com/adambarreiro/go-vcloud-director/v2 v2.17.0-alpha.1.0.20231122110403-a2835e0d3940/go.mod h1:QPxGFgrUcSyzy9IlpwDE4UNT3tsOy2047tJOPEJ4nlw= +github.com/adambarreiro/go-vcloud-director/v2 v2.17.0-alpha.1.0.20231124133658-498565f7a0cc h1:ooLo9N4kRTeFf/ik/Hx6E0L0GscGG4ZyQI3IT8gJCnE= +github.com/adambarreiro/go-vcloud-director/v2 v2.17.0-alpha.1.0.20231124133658-498565f7a0cc/go.mod h1:QPxGFgrUcSyzy9IlpwDE4UNT3tsOy2047tJOPEJ4nlw= github.com/agext/levenshtein v1.2.2 h1:0S/Yg6LYmFJ5stwQeRp6EeOcCbj7xiqQSdNelsXvaqE= github.com/agext/levenshtein v1.2.2/go.mod h1:JEDfjyjHDjOF/1e4FlBE/PkbqA9OfWu2ki2W0IB5558= github.com/apparentlymart/go-textseg/v12 v12.0.0/go.mod h1:S/4uRK2UtaQttw1GenVJEynmyUenKwP++x/+DdGV/Ec= diff --git a/vcd/metadata_openapi.go b/vcd/metadata_openapi.go index 8b0c71ba4..6ac8be00d 100644 --- a/vcd/metadata_openapi.go +++ b/vcd/metadata_openapi.go @@ -4,6 +4,7 @@ import ( "fmt" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/validation" + "github.com/vmware/go-vcloud-director/v2/govcd" "github.com/vmware/go-vcloud-director/v2/types/v56" "github.com/vmware/go-vcloud-director/v2/util" "reflect" @@ -125,11 +126,10 @@ func openApiMetadataEntryResourceSchema(resourceType string) *schema.Schema { // openApiMetadataCompatible allows to consider all structs that implement OpenAPI metadata handling to be the same type type openApiMetadataCompatible interface { - GetMetadata() ([]*types.OpenApiMetadataEntry, error) - GetMetadataByKey(namespace, key string) (*types.OpenApiMetadataEntry, error) - AddMetadata(metadataEntry types.OpenApiMetadataEntry) (*types.OpenApiMetadataEntry, error) - UpdateMetadata(namespace, key string, value interface{}) (*types.OpenApiMetadataEntry, error) - DeleteMetadata(namespace, key string) error + GetMetadata() ([]*govcd.OpenApiMetadataEntry, error) + GetMetadataByKey(domain, namespace, key string) (*govcd.OpenApiMetadataEntry, error) + GetMetadataById(id string) (*govcd.OpenApiMetadataEntry, error) + AddMetadata(metadataEntry types.OpenApiMetadataEntry) (*govcd.OpenApiMetadataEntry, error) } // createOrUpdateOpenApiMetadataEntryInVcd creates or updates OpenAPI metadata entries in VCD for the given resource, only if the attribute @@ -146,14 +146,22 @@ func createOrUpdateOpenApiMetadataEntryInVcd(d *schema.ResourceData, resource op } for _, entry := range metadataToDelete { - err = resource.DeleteMetadata(entry.KeyValue.Namespace, entry.KeyValue.Key) + toDelete, err := resource.GetMetadataById(entry.ID) // Refreshes ETags + if err != nil { + return fmt.Errorf("error reading metadata with namespace '%s' and key '%s': %s", entry.KeyValue.Namespace, entry.KeyValue.Key, err) + } + err = toDelete.Delete() if err != nil { return fmt.Errorf("error deleting metadata with namespace '%s' and key '%s': %s", entry.KeyValue.Namespace, entry.KeyValue.Key, err) } } for _, entry := range metadataToUpdate { - _, err = resource.UpdateMetadata(entry.KeyValue.Namespace, entry.KeyValue.Key, entry.KeyValue.Value.Value) + toUpdate, err := resource.GetMetadataById(entry.ID) // Refreshes ETags + if err != nil { + return fmt.Errorf("error reading metadata with namespace '%s' and key '%s': %s", entry.KeyValue.Namespace, entry.KeyValue.Key, err) + } + err = toUpdate.Update(entry.KeyValue.Value.Value, entry.IsPersistent) if err != nil { return fmt.Errorf("error updating metadata with namespace '%s' and key '%s': %s", entry.KeyValue.Namespace, entry.KeyValue.Key, err) } @@ -277,26 +285,26 @@ func updateOpenApiMetadataInState(d *schema.ResourceData, receiverObject openApi for i, metadataEntryFromVcd := range allMetadata { // We need to set the correct type, otherwise saving the state will fail value := "" - switch metadataEntryFromVcd.KeyValue.Value.Type { + switch metadataEntryFromVcd.MetadataEntry.KeyValue.Value.Type { case types.OpenApiMetadataBooleanEntry: - value = fmt.Sprintf("%t", metadataEntryFromVcd.KeyValue.Value.Value.(bool)) + value = fmt.Sprintf("%t", metadataEntryFromVcd.MetadataEntry.KeyValue.Value.Value.(bool)) case types.OpenApiMetadataNumberEntry: - value = fmt.Sprintf("%.0f", metadataEntryFromVcd.KeyValue.Value.Value.(float64)) + value = fmt.Sprintf("%.0f", metadataEntryFromVcd.MetadataEntry.KeyValue.Value.Value.(float64)) case types.OpenApiMetadataStringEntry: - value = metadataEntryFromVcd.KeyValue.Value.Value.(string) + value = metadataEntryFromVcd.MetadataEntry.KeyValue.Value.Value.(string) default: - return fmt.Errorf("not supported metadata type %s", metadataEntryFromVcd.KeyValue.Value.Type) + return fmt.Errorf("not supported metadata type %s", metadataEntryFromVcd.MetadataEntry.KeyValue.Value.Type) } metadataEntry := map[string]interface{}{ - "id": metadataEntryFromVcd.ID, - "key": metadataEntryFromVcd.KeyValue.Key, - "readonly": metadataEntryFromVcd.IsReadOnly, - "domain": metadataEntryFromVcd.KeyValue.Domain, - "namespace": metadataEntryFromVcd.KeyValue.Namespace, - "type": metadataEntryFromVcd.KeyValue.Value.Type, + "id": metadataEntryFromVcd.MetadataEntry.ID, + "key": metadataEntryFromVcd.MetadataEntry.KeyValue.Key, + "readonly": metadataEntryFromVcd.MetadataEntry.IsReadOnly, + "domain": metadataEntryFromVcd.MetadataEntry.KeyValue.Domain, + "namespace": metadataEntryFromVcd.MetadataEntry.KeyValue.Namespace, + "type": metadataEntryFromVcd.MetadataEntry.KeyValue.Value.Type, "value": value, - "persistent": metadataEntryFromVcd.IsPersistent, + "persistent": metadataEntryFromVcd.MetadataEntry.IsPersistent, } metadata[i] = metadataEntry } From cf50f700983ce15a3b801361293c6d6e25b1cc20 Mon Sep 17 00:00:00 2001 From: abarreiro Date: Fri, 24 Nov 2023 14:57:56 +0100 Subject: [PATCH 35/39] Add limit of 50 Signed-off-by: abarreiro --- vcd/metadata_openapi.go | 1 + 1 file changed, 1 insertion(+) diff --git a/vcd/metadata_openapi.go b/vcd/metadata_openapi.go index 6ac8be00d..71b98a83e 100644 --- a/vcd/metadata_openapi.go +++ b/vcd/metadata_openapi.go @@ -72,6 +72,7 @@ func openApiMetadataEntryResourceSchema(resourceType string) *schema.Schema { Type: schema.TypeSet, Optional: true, Description: fmt.Sprintf("Metadata entries for the given %s", resourceType), + MaxItems: 50, // As per the documentation Elem: &schema.Resource{ Schema: map[string]*schema.Schema{ "id": { From 4758936f069f701b9da872d212915df2bae4206a Mon Sep 17 00:00:00 2001 From: abarreiro Date: Fri, 24 Nov 2023 15:09:14 +0100 Subject: [PATCH 36/39] # Signed-off-by: abarreiro --- vcd/metadata_openapi.go | 9 ++++----- website/docs/r/rde.html.markdown | 9 ++++----- 2 files changed, 8 insertions(+), 10 deletions(-) diff --git a/vcd/metadata_openapi.go b/vcd/metadata_openapi.go index 71b98a83e..0cd845ebe 100644 --- a/vcd/metadata_openapi.go +++ b/vcd/metadata_openapi.go @@ -203,15 +203,14 @@ func getOpenApiMetadataOperations(oldMetadata []interface{}, newMetadata []inter if reflect.DeepEqual(oldEntry, newEntry) { continue } - // If a metadata property that is not "Value" is changed, it needs to be recreated - if oldEntry.IsReadOnly != newEntry.IsReadOnly || oldEntry.IsPersistent != newEntry.IsPersistent || - oldEntry.KeyValue.Namespace != newEntry.KeyValue.Namespace || oldEntry.KeyValue.Domain != newEntry.KeyValue.Domain || - oldEntry.KeyValue.Value.Type != newEntry.KeyValue.Value.Type { + // If a metadata property that is not "Value" or "IsPersistent" is changed, it needs to be recreated + if oldEntry.IsReadOnly != newEntry.IsReadOnly || oldEntry.KeyValue.Namespace != newEntry.KeyValue.Namespace || + oldEntry.KeyValue.Domain != newEntry.KeyValue.Domain || oldEntry.KeyValue.Value.Type != newEntry.KeyValue.Value.Type { util.Logger.Printf("[DEBUG] entry with namespace '%s' and key '%s' is being deleted and re-created", oldEntry.KeyValue.Namespace, oldEntry.KeyValue.Key) metadataToRemove = append(metadataToRemove, oldMetadataEntries[newNamespacedKey]) metadataToCreate = append(metadataToCreate, newMetadataEntries[newNamespacedKey]) } else { - // Only "Value" is changed, it can be updated + // Only "Value" / "IsPersistent" is changed, it can be updated metadataToUpdateMap[newNamespacedKey] = newEntry } diff --git a/website/docs/r/rde.html.markdown b/website/docs/r/rde.html.markdown index 403b1ba9c..9493fe577 100644 --- a/website/docs/r/rde.html.markdown +++ b/website/docs/r/rde.html.markdown @@ -205,13 +205,12 @@ The `metadata_entry` is a set of metadata entries that have the following struct * `domain` - (Optional) Only meaningful for providers. Allows them to share entries with their tenants. Currently, accepted values are: `TENANT`, `PROVIDER`. Defaults to `TENANT`. Updating this value forces a re-creation of the metadata entry. * `readonly` - (Optional) `true` if the metadata entry is read only. Defaults to `false`. Updating this value forces a re-creation of the metadata entry. -* `persistent` - (Optional) `true` if the metadata is persistent. Persistent entries can be copied over on some entity operation - (e.g. Creating a copy of a VDC, capturing a vApp to a template, instantiating a catalog item as a VM...). Defaults to `false`. - Updating this value forces a re-creation of the metadata entry. +* `persistent` - (Optional) `true` if the metadata is persistent. Persistent entries can be copied over on some entity operation. + Right now it doesn't have any effect. * `id` - (Computed) Read-only identifier for this metadata entry. -The only attribute that supports updates-in-place for a given metadata entry is `value`. Updating any other value will re-create the -metadata entry. +The only attributes that support updates-in-place for a given metadata entry is `value` and `persistent`. +Updating any other value will re-create the metadata entry. Example: From 2b45dcc4091a1f19b49a86d7cfd53c24047870d1 Mon Sep 17 00:00:00 2001 From: abarreiro Date: Fri, 24 Nov 2023 15:11:41 +0100 Subject: [PATCH 37/39] Changelog Signed-off-by: abarreiro --- .changes/v3.11.0/1018-improvements.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.changes/v3.11.0/1018-improvements.md b/.changes/v3.11.0/1018-improvements.md index b2276fc7f..ce09f000d 100644 --- a/.changes/v3.11.0/1018-improvements.md +++ b/.changes/v3.11.0/1018-improvements.md @@ -1 +1,2 @@ -* Add `metadata_entry` attribute to `vcd_rde` resource and data source to manage metadata entries in Runtime Defined Entities [GH-1018] +* Add `metadata_entry` attribute to `vcd_rde` resource and data source to manage metadata entries of type + `String`, `Number` and `Bool` in Runtime Defined Entities [GH-1018] From ab7804972e475fdde2ed4055a66f64d62a45e52c Mon Sep 17 00:00:00 2001 From: abarreiro Date: Fri, 24 Nov 2023 15:40:40 +0100 Subject: [PATCH 38/39] Fix. Tests pass Signed-off-by: abarreiro --- vcd/metadata_openapi.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/vcd/metadata_openapi.go b/vcd/metadata_openapi.go index 0cd845ebe..02801eece 100644 --- a/vcd/metadata_openapi.go +++ b/vcd/metadata_openapi.go @@ -147,7 +147,7 @@ func createOrUpdateOpenApiMetadataEntryInVcd(d *schema.ResourceData, resource op } for _, entry := range metadataToDelete { - toDelete, err := resource.GetMetadataById(entry.ID) // Refreshes ETags + toDelete, err := resource.GetMetadataByKey(entry.KeyValue.Domain, entry.KeyValue.Namespace, entry.KeyValue.Key) // Refreshes ETags if err != nil { return fmt.Errorf("error reading metadata with namespace '%s' and key '%s': %s", entry.KeyValue.Namespace, entry.KeyValue.Key, err) } @@ -158,7 +158,7 @@ func createOrUpdateOpenApiMetadataEntryInVcd(d *schema.ResourceData, resource op } for _, entry := range metadataToUpdate { - toUpdate, err := resource.GetMetadataById(entry.ID) // Refreshes ETags + toUpdate, err := resource.GetMetadataByKey(entry.KeyValue.Domain, entry.KeyValue.Namespace, entry.KeyValue.Key) // Refreshes ETags if err != nil { return fmt.Errorf("error reading metadata with namespace '%s' and key '%s': %s", entry.KeyValue.Namespace, entry.KeyValue.Key, err) } From 04089e11b23ae4994ab3b104c6ca67c16a81331d Mon Sep 17 00:00:00 2001 From: abarreiro Date: Tue, 28 Nov 2023 09:49:38 +0100 Subject: [PATCH 39/39] Finish PR, dump gosdk Signed-off-by: abarreiro --- go.mod | 4 +--- go.sum | 4 ++-- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/go.mod b/go.mod index cb71850b9..0d262f925 100644 --- a/go.mod +++ b/go.mod @@ -7,7 +7,7 @@ require ( github.com/hashicorp/go-version v1.6.0 github.com/hashicorp/terraform-plugin-sdk/v2 v2.29.0 github.com/kr/pretty v0.2.1 - github.com/vmware/go-vcloud-director/v2 v2.22.0-alpha.13 + github.com/vmware/go-vcloud-director/v2 v2.22.0-alpha.14 ) require ( @@ -65,5 +65,3 @@ require ( google.golang.org/grpc v1.57.1 // indirect google.golang.org/protobuf v1.31.0 // indirect ) - -replace github.com/vmware/go-vcloud-director/v2 => github.com/adambarreiro/go-vcloud-director/v2 v2.17.0-alpha.1.0.20231124145647-29e55026cff9 diff --git a/go.sum b/go.sum index fb97fe110..af6d9b0f0 100644 --- a/go.sum +++ b/go.sum @@ -3,8 +3,6 @@ github.com/Microsoft/go-winio v0.6.1 h1:9/kr64B9VUZrLm5YYwbGtUJnMgqWVOdUAXu6Migc github.com/ProtonMail/go-crypto v0.0.0-20230717121422-5aa5874ade95 h1:KLq8BE0KwCL+mmXnjLWEAOYO+2l2AE4YMmqG1ZpZHBs= github.com/ProtonMail/go-crypto v0.0.0-20230717121422-5aa5874ade95/go.mod h1:EjAoLdwvbIOoOQr3ihjnSoLZRtE8azugULFRteWMNc0= github.com/acomagu/bufpipe v1.0.4 h1:e3H4WUzM3npvo5uv95QuJM3cQspFNtFBzvJ2oNjKIDQ= -github.com/adambarreiro/go-vcloud-director/v2 v2.17.0-alpha.1.0.20231124145647-29e55026cff9 h1:OESdusW7UQxmBajhO0huQsIDjuuKVlrVaHs5X02kylE= -github.com/adambarreiro/go-vcloud-director/v2 v2.17.0-alpha.1.0.20231124145647-29e55026cff9/go.mod h1:QPxGFgrUcSyzy9IlpwDE4UNT3tsOy2047tJOPEJ4nlw= github.com/agext/levenshtein v1.2.2 h1:0S/Yg6LYmFJ5stwQeRp6EeOcCbj7xiqQSdNelsXvaqE= github.com/agext/levenshtein v1.2.2/go.mod h1:JEDfjyjHDjOF/1e4FlBE/PkbqA9OfWu2ki2W0IB5558= github.com/apparentlymart/go-textseg/v12 v12.0.0/go.mod h1:S/4uRK2UtaQttw1GenVJEynmyUenKwP++x/+DdGV/Ec= @@ -126,6 +124,8 @@ github.com/vmihailenco/msgpack/v5 v5.3.5 h1:5gO0H1iULLWGhs2H5tbAHIZTV8/cYafcFOr9 github.com/vmihailenco/msgpack/v5 v5.3.5/go.mod h1:7xyJ9e+0+9SaZT0Wt1RGleJXzli6Q/V5KbhBonMG9jc= github.com/vmihailenco/tagparser/v2 v2.0.0 h1:y09buUbR+b5aycVFQs/g70pqKVZNBmxwAhO7/IwNM9g= github.com/vmihailenco/tagparser/v2 v2.0.0/go.mod h1:Wri+At7QHww0WTrCBeu4J6bNtoV6mEfg5OIWRZA9qds= +github.com/vmware/go-vcloud-director/v2 v2.22.0-alpha.14 h1:xsyLC+kVS57PeC4HoVXk798ScVryDs+a2ym3t1BPVB0= +github.com/vmware/go-vcloud-director/v2 v2.22.0-alpha.14/go.mod h1:QPxGFgrUcSyzy9IlpwDE4UNT3tsOy2047tJOPEJ4nlw= github.com/xanzy/ssh-agent v0.3.3 h1:+/15pJfg/RsTxqYcX6fHqOXZwwMP+2VyYWJeWM2qQFM= github.com/yuin/goldmark v1.4.13/go.mod h1:6yULJ656Px+3vBD8DxQVa3kxgyrAnzto9xy5taEt/CY= github.com/zclconf/go-cty v1.14.0 h1:/Xrd39K7DXbHzlisFP9c4pHao4yyf+/Ug9LEz+Y/yhc=