From af98aaa9b2d6b2cb0b1b9bf65bcc44734014ab51 Mon Sep 17 00:00:00 2001 From: Louis Ruch Date: Mon, 15 Nov 2021 10:42:00 -0800 Subject: [PATCH] chore(targets): remove deprecated fields (#150) --- docs/resources/target.md | 2 - internal/provider/resource_target.go | 117 ++---------- internal/provider/resource_target_test.go | 219 ---------------------- 3 files changed, 12 insertions(+), 326 deletions(-) diff --git a/docs/resources/target.md b/docs/resources/target.md index 7201a133..cdc0982c 100644 --- a/docs/resources/target.md +++ b/docs/resources/target.md @@ -105,11 +105,9 @@ resource "boundary_target" "foo" { ### Optional -- **application_credential_library_ids** (Set of String, Deprecated) A list of application credential library ID's. - **application_credential_source_ids** (Set of String) A list of application credential source ID's. - **default_port** (Number) The default port for this target. - **description** (String) The target description. -- **host_set_ids** (Set of String, Deprecated) A list of host set ID's. - **host_source_ids** (Set of String) A list of host source ID's. - **name** (String) The target name. Defaults to the resource name. - **session_connection_limit** (Number) diff --git a/internal/provider/resource_target.go b/internal/provider/resource_target.go index 957072dc..c693c62d 100644 --- a/internal/provider/resource_target.go +++ b/internal/provider/resource_target.go @@ -67,35 +67,17 @@ func resourceTarget() *schema.Resource { Type: schema.TypeInt, Optional: true, }, - "host_set_ids": { - Description: "A list of host set ID's.", - Type: schema.TypeSet, - Optional: true, - Deprecated: "Please use 'host_source_ids' instead", - ConflictsWith: []string{targetHostSourceIdsKey}, - Elem: &schema.Schema{Type: schema.TypeString}, - }, - "application_credential_library_ids": { - Description: "A list of application credential library ID's.", - Type: schema.TypeSet, - Optional: true, - Deprecated: "Please use 'application_credential_source_ids' instead", - ConflictsWith: []string{targetApplicationCredentialSourceIdsKey}, - Elem: &schema.Schema{Type: schema.TypeString}, - }, targetHostSourceIdsKey: { - Description: "A list of host source ID's.", - Type: schema.TypeSet, - Optional: true, - ConflictsWith: []string{"host_set_ids"}, - Elem: &schema.Schema{Type: schema.TypeString}, + Description: "A list of host source ID's.", + Type: schema.TypeSet, + Optional: true, + Elem: &schema.Schema{Type: schema.TypeString}, }, targetApplicationCredentialSourceIdsKey: { - Description: "A list of application credential source ID's.", - Type: schema.TypeSet, - Optional: true, - ConflictsWith: []string{"application_credential_library_ids"}, - Elem: &schema.Schema{Type: schema.TypeString}, + Description: "A list of application credential source ID's.", + Type: schema.TypeSet, + Optional: true, + Elem: &schema.Schema{Type: schema.TypeString}, }, targetSessionMaxSecondsKey: { Type: schema.TypeInt, @@ -129,23 +111,11 @@ func setFromTargetResponseMap(d *schema.ResourceData, raw map[string]interface{} if err := d.Set(TypeKey, raw["type"]); err != nil { return err } - if _, ok := d.GetOk("host_set_ids"); ok { - if err := d.Set("host_set_ids", raw["host_set_ids"]); err != nil { - return err - } - } else { - if err := d.Set(targetHostSourceIdsKey, raw["host_source_ids"]); err != nil { - return err - } + if err := d.Set(targetHostSourceIdsKey, raw["host_source_ids"]); err != nil { + return err } - if _, ok := d.GetOk("application_credential_library_ids"); ok { - if err := d.Set("application_credential_library_ids", raw["application_credential_library_ids"]); err != nil { - return err - } - } else { - if err := d.Set(targetApplicationCredentialSourceIdsKey, raw["application_credential_source_ids"]); err != nil { - return err - } + if err := d.Set(targetApplicationCredentialSourceIdsKey, raw["application_credential_source_ids"]); err != nil { + return err } if err := d.Set(targetSessionMaxSecondsKey, raw["session_max_seconds"]); err != nil { return err @@ -245,15 +215,6 @@ func resourceTargetCreate(ctx context.Context, d *schema.ResourceData, meta inte } } - // TODO: remove when host_set_ids is fully deprecated - if hostSetIdsVal, ok := d.GetOk("host_set_ids"); ok { - list := hostSetIdsVal.(*schema.Set).List() - hostSourceIds = make([]string, 0, len(list)) - for _, i := range list { - hostSourceIds = append(hostSourceIds, i.(string)) - } - } - var credentialSourceIds []string if credentialSourceIdsVal, ok := d.GetOk(targetApplicationCredentialSourceIdsKey); ok { list := credentialSourceIdsVal.(*schema.Set).List() @@ -263,15 +224,6 @@ func resourceTargetCreate(ctx context.Context, d *schema.ResourceData, meta inte } } - // TODO: remove when application_credential_library_ids is fully deprecated - if credentialLibIdsVal, ok := d.GetOk("application_credential_library_ids"); ok { - list := credentialLibIdsVal.(*schema.Set).List() - credentialSourceIds = make([]string, 0, len(list)) - for _, i := range list { - credentialSourceIds = append(credentialSourceIds, i.(string)) - } - } - workerFilterVal, ok := d.GetOk(targetWorkerFilterKey) if ok { workerFilterStr := workerFilterVal.(string) @@ -476,24 +428,6 @@ func resourceTargetUpdate(ctx context.Context, d *schema.ResourceData, meta inte } } - // // TODO: remove when host_set_ids is fully deprecated - if d.HasChange("host_set_ids") { - var hostSourceIds []string - if hostSourceIdsVal, ok := d.GetOk("host_set_ids"); ok { - hostSources := hostSourceIdsVal.(*schema.Set).List() - for _, hostSource := range hostSources { - hostSourceIds = append(hostSourceIds, hostSource.(string)) - } - } - _, err := tc.SetHostSets(ctx, d.Id(), 0, hostSourceIds, targets.WithAutomaticVersioning(true)) - if err != nil { - return diag.Errorf("error updating host sources in target: %v", err) - } - if err := d.Set("host_set_ids", hostSourceIds); err != nil { - return diag.FromErr(err) - } - } - // The above calls may not actually happen, so we use d.Id() and automatic // versioning here if d.HasChange(targetApplicationCredentialSourceIdsKey) { @@ -522,33 +456,6 @@ func resourceTargetUpdate(ctx context.Context, d *schema.ResourceData, meta inte } } - // TODO: remove when application_credential_library_ids is fully deprecated - if d.HasChange("application_credential_library_ids") { - var credentialSourceIds []string - if credentialSourceIdsVal, ok := d.GetOk("application_credential_library_ids"); ok { - credSourceIds := credentialSourceIdsVal.(*schema.Set).List() - for _, credSourceId := range credSourceIds { - credentialSourceIds = append(credentialSourceIds, credSourceId.(string)) - } - } - - opts := []targets.Option{ - targets.WithAutomaticVersioning(true), - targets.DefaultApplicationCredentialLibraryIds(), - } - if len(credentialSourceIds) > 0 { - opts = append(opts, targets.WithApplicationCredentialLibraryIds(credentialSourceIds)) - } - - _, err := tc.SetCredentialLibraries(ctx, d.Id(), 0, opts...) - if err != nil { - return diag.Errorf("error updating credential sources in target: %v", err) - } - if err := d.Set("application_credential_library_ids", credentialSourceIds); err != nil { - return diag.FromErr(err) - } - } - return nil } diff --git a/internal/provider/resource_target_test.go b/internal/provider/resource_target_test.go index f2a62d33..159d76eb 100644 --- a/internal/provider/resource_target_test.go +++ b/internal/provider/resource_target_test.go @@ -130,44 +130,6 @@ resource "boundary_target" "foo" { session_connection_limit = 7 worker_filter = "type == \"bar\"" }`, fooTargetDescriptionUpdate) - - // TODO: remove this when host_set_ids and application_credential_library_ids - // are fully deprecated - fooTargetDeprecatedFields = ` -resource "boundary_target" "foo" { - name = "test_deprecated" - type = "tcp" - scope_id = boundary_scope.proj1.id - host_set_ids = [ - boundary_host_set.foo.id - ] - application_credential_library_ids = [ - boundary_credential_library_vault.foo.id - ] - depends_on = [boundary_role.proj1_admin] -}` - - fooTargetDeprecatedFieldsUpdate = ` -resource "boundary_target" "foo" { - name = "test_deprecated" - type = "tcp" - scope_id = boundary_scope.proj1.id - host_set_ids = [ - boundary_host_set.bar.id - ] - application_credential_library_ids = [ - boundary_credential_library_vault.bar.id - ] - depends_on = [boundary_role.proj1_admin] -}` - - fooTargetDeprecatedFieldsUnset = ` -resource "boundary_target" "foo" { - name = "test_deprecated" - type = "tcp" - scope_id = boundary_scope.proj1.id - depends_on = [boundary_role.proj1_admin] -}` ) func TestAccTarget(t *testing.T) { @@ -205,8 +167,6 @@ func TestAccTarget(t *testing.T) { resource.TestCheckResourceAttr("boundary_target.foo", targetWorkerFilterKey, `type == "foo"`), testAccCheckTargetResourceHostSource(provider, "boundary_target.foo", []string{"boundary_host_set.foo"}), testAccCheckTargetResourceAppCredSources(provider, "boundary_target.foo", []string{"boundary_credential_library_vault.foo"}), - testAccCheckTargetResourceHostSet(provider, "boundary_target.foo", []string{"boundary_host_set.foo"}), - testAccCheckTargetResourceAppCredLibs(provider, "boundary_target.foo", []string{"boundary_credential_library_vault.foo"}), ), }, importStep("boundary_target.foo"), @@ -222,8 +182,6 @@ func TestAccTarget(t *testing.T) { resource.TestCheckResourceAttr("boundary_target.foo", targetWorkerFilterKey, `type == "bar"`), testAccCheckTargetResourceHostSource(provider, "boundary_target.foo", []string{"boundary_host_set.bar"}), testAccCheckTargetResourceAppCredSources(provider, "boundary_target.foo", []string{"boundary_credential_library_vault.bar"}), - testAccCheckTargetResourceHostSet(provider, "boundary_target.foo", []string{"boundary_host_set.bar"}), - testAccCheckTargetResourceAppCredLibs(provider, "boundary_target.foo", []string{"boundary_credential_library_vault.bar"}), ), }, importStep("boundary_target.foo"), @@ -239,8 +197,6 @@ func TestAccTarget(t *testing.T) { resource.TestCheckResourceAttr("boundary_target.foo", targetWorkerFilterKey, `type == "bar"`), testAccCheckTargetResourceHostSource(provider, "boundary_target.foo", nil), testAccCheckTargetResourceAppCredSources(provider, "boundary_target.foo", nil), - testAccCheckTargetResourceHostSet(provider, "boundary_target.foo", nil), - testAccCheckTargetResourceAppCredLibs(provider, "boundary_target.foo", nil), ), }, importStep("boundary_target.foo"), @@ -248,65 +204,6 @@ func TestAccTarget(t *testing.T) { }) } -func TestAccTargetDeprecatedFields(t *testing.T) { - tc := controller.NewTestController(t, tcConfig...) - - defer tc.Shutdown() - url := tc.ApiAddrs()[0] - - vc := vault.NewTestVaultServer(t) - _, token := vc.CreateToken(t) - credStoreRes := vaultCredStoreResource(vc, - vaultCredStoreName, - vaultCredStoreDesc, - vaultCredStoreNamespace, - "www.original.com", - token, - true) - - var provider *schema.Provider - resource.Test(t, resource.TestCase{ - IsUnitTest: true, - ProviderFactories: providerFactories(&provider), - CheckDestroy: testAccCheckTargetResourceDestroy(t, provider), - Steps: []resource.TestStep{ - { - // test create - Config: testConfig(url, fooOrg, firstProjectFoo, credStoreRes, fooBarCredLibs, fooBarHostSet, fooTargetDeprecatedFields), - Check: resource.ComposeTestCheckFunc( - testAccCheckTargetResourceExists(provider, "boundary_target.foo"), - testAccCheckTargetResourceHostSource(provider, "boundary_target.foo", []string{"boundary_host_set.foo"}), - testAccCheckTargetResourceAppCredSources(provider, "boundary_target.foo", []string{"boundary_credential_library_vault.foo"}), - testAccCheckTargetResourceHostSet(provider, "boundary_target.foo", []string{"boundary_host_set.foo"}), - testAccCheckTargetResourceAppCredLibs(provider, "boundary_target.foo", []string{"boundary_credential_library_vault.foo"}), - ), - }, - { - // test update - Config: testConfig(url, fooOrg, firstProjectFoo, credStoreRes, fooBarCredLibs, fooBarHostSet, fooTargetDeprecatedFieldsUpdate), - Check: resource.ComposeTestCheckFunc( - testAccCheckTargetResourceExists(provider, "boundary_target.foo"), - testAccCheckTargetResourceHostSource(provider, "boundary_target.foo", []string{"boundary_host_set.bar"}), - testAccCheckTargetResourceAppCredSources(provider, "boundary_target.foo", []string{"boundary_credential_library_vault.bar"}), - testAccCheckTargetResourceHostSet(provider, "boundary_target.foo", []string{"boundary_host_set.bar"}), - testAccCheckTargetResourceAppCredLibs(provider, "boundary_target.foo", []string{"boundary_credential_library_vault.bar"}), - ), - }, - { - // test unset hosts and cred sources - Config: testConfig(url, fooOrg, firstProjectFoo, credStoreRes, fooBarCredLibs, fooBarHostSet, fooTargetDeprecatedFieldsUnset), - Check: resource.ComposeTestCheckFunc( - testAccCheckTargetResourceExists(provider, "boundary_target.foo"), - testAccCheckTargetResourceHostSource(provider, "boundary_target.foo", nil), - testAccCheckTargetResourceAppCredSources(provider, "boundary_target.foo", nil), - testAccCheckTargetResourceHostSet(provider, "boundary_target.foo", nil), - testAccCheckTargetResourceAppCredLibs(provider, "boundary_target.foo", nil), - ), - }, - }, - }) -} - func testAccCheckTargetResourceHostSource(testProvider *schema.Provider, name string, hostSources []string) resource.TestCheckFunc { return func(s *terraform.State) error { rs, ok := s.RootModule().Resources[name] @@ -421,122 +318,6 @@ func testAccCheckTargetResourceAppCredSources(testProvider *schema.Provider, nam } } -// TODO: remove when host_set_ids is fully deprecated -func testAccCheckTargetResourceHostSet(testProvider *schema.Provider, name string, hostSets []string) resource.TestCheckFunc { - return func(s *terraform.State) error { - rs, ok := s.RootModule().Resources[name] - if !ok { - return fmt.Errorf("target resource not found: %s", name) - } - - id := rs.Primary.ID - if id == "" { - return fmt.Errorf("target resource ID is not set") - } - - // ensure host sets are declared in state - var hostSetIDs []string - for _, hostSetResourceID := range hostSets { - hs, ok := s.RootModule().Resources[hostSetResourceID] - if !ok { - return fmt.Errorf("host set resource not found: %s", hostSetResourceID) - } - - hostSetID := hs.Primary.ID - if id == "" { - return fmt.Errorf("host set resource ID not set") - } - - hostSetIDs = append(hostSetIDs, hostSetID) - } - - // check boundary to ensure it matches - md := testProvider.Meta().(*metaData) - tgtsClient := targets.NewClient(md.client) - - t, err := tgtsClient.Read(context.Background(), id) - if err != nil { - return fmt.Errorf("Got an error when reading target %q: %v", id, err) - } - - if len(t.Item.HostSetIds) != len(hostSetIDs) { - return fmt.Errorf("tf state and boundary have different number of host sets") - } - - for _, stateHostSetId := range t.Item.HostSetIds { - ok := false - for _, gotHostSetID := range hostSetIDs { - if gotHostSetID == stateHostSetId { - ok = true - } - } - if !ok { - return fmt.Errorf("host set id in state not set in boundary: %s", stateHostSetId) - } - } - - return nil - } -} - -// TODO: remove when application_credential_library_ids is fully deprecated -func testAccCheckTargetResourceAppCredLibs(testProvider *schema.Provider, name string, credLibs []string) resource.TestCheckFunc { - return func(s *terraform.State) error { - rs, ok := s.RootModule().Resources[name] - if !ok { - return fmt.Errorf("target resource not found: %s", name) - } - - id := rs.Primary.ID - if id == "" { - return fmt.Errorf("target resource ID is not set") - } - - // ensure host sets are declared in state - var credLibIDs []string - for _, credLibResourceID := range credLibs { - cl, ok := s.RootModule().Resources[credLibResourceID] - if !ok { - return fmt.Errorf("credential library resource not found: %s", credLibResourceID) - } - - credLibID := cl.Primary.ID - if id == "" { - return fmt.Errorf("credential library resource ID not set") - } - - credLibIDs = append(credLibIDs, credLibID) - } - - // check boundary to ensure it matches - md := testProvider.Meta().(*metaData) - tgtsClient := targets.NewClient(md.client) - - t, err := tgtsClient.Read(context.Background(), id) - if err != nil { - return fmt.Errorf("Got an error when reading target %q: %v", id, err) - } - - if len(t.Item.ApplicationCredentialLibraryIds) != len(credLibIDs) { - return fmt.Errorf("tf state and boundary have different number of application credential libraries") - } - - for _, stateCredLibId := range t.Item.ApplicationCredentialLibraryIds { - ok := false - for _, gotCredLibID := range credLibIDs { - if gotCredLibID == stateCredLibId { - ok = true - } - } - if !ok { - return fmt.Errorf("application credential library id in state not set in boundary: %s", stateCredLibId) - } - } - - return nil - } -} - func testAccCheckTargetResourceExists(testProvider *schema.Provider, name string) resource.TestCheckFunc { return func(s *terraform.State) error { rs, ok := s.RootModule().Resources[name]