From acb5f03e5d30a994e3d8631da60f3ca5ea11c882 Mon Sep 17 00:00:00 2001 From: JM Faircloth Date: Thu, 14 Dec 2023 10:14:27 -0600 Subject: [PATCH 1/4] move new fw resources to internal/vault/ dir --- internal/provider/fwprovider/provider.go | 4 +- internal/providertest/providertest.go | 40 +++++++++++++++++ internal/vault/auth/README.md | 3 ++ internal/vault/secrets/README.md | 3 ++ internal/vault/sys/README.md | 3 ++ internal/{ => vault}/sys/password_policy.go | 0 internal/vault/sys/password_policy_test.go | 48 +++++++++++++++++++++ 7 files changed, 99 insertions(+), 2 deletions(-) create mode 100644 internal/providertest/providertest.go create mode 100644 internal/vault/auth/README.md create mode 100644 internal/vault/secrets/README.md create mode 100644 internal/vault/sys/README.md rename internal/{ => vault}/sys/password_policy.go (100%) create mode 100644 internal/vault/sys/password_policy_test.go diff --git a/internal/provider/fwprovider/provider.go b/internal/provider/fwprovider/provider.go index 16a910b2b..904fe38b8 100644 --- a/internal/provider/fwprovider/provider.go +++ b/internal/provider/fwprovider/provider.go @@ -12,7 +12,7 @@ import ( "github.com/hashicorp/terraform-plugin-framework/schema/validator" "github.com/hashicorp/terraform-provider-vault/internal/consts" - "github.com/hashicorp/terraform-provider-vault/internal/sys" + "github.com/hashicorp/terraform-provider-vault/internal/vault/sys" ) // Ensure the implementation satisfies the provider.Provider interface @@ -47,7 +47,7 @@ func (p *fwprovider) Metadata(ctx context.Context, req provider.MetadataRequest, // // Schema is called during validate, plan and apply. func (p *fwprovider) Schema(ctx context.Context, req provider.SchemaRequest, resp *provider.SchemaResponse) { - // TODO(JM): This schema must match exactly to the SDKv2 provider's schema + // This schema must match exactly to the SDKv2 provider's schema resp.Schema = schema.Schema{ Attributes: map[string]schema.Attribute{ // Not `Required` but must be set via config or env. Otherwise we diff --git a/internal/providertest/providertest.go b/internal/providertest/providertest.go new file mode 100644 index 000000000..61a17dd09 --- /dev/null +++ b/internal/providertest/providertest.go @@ -0,0 +1,40 @@ +package providertest + +import ( + "context" + + "github.com/hashicorp/terraform-plugin-go/tfprotov5" + "github.com/hashicorp/terraform-provider-vault/vault" +) + +// ProtoV5ProviderFactories is a static map containing only the main provider instance +var ( + ProtoV5ProviderFactories map[string]func() (tfprotov5.ProviderServer, error) = protoV5ProviderFactoriesInit(context.Background(), "vault") +) + +// testAccProtoV5ProviderFactories will return a map of provider servers +// suitable for use as a resource.TestStep.ProtoV5ProviderFactories. +// +// When multiplexing providers, the schema and configuration handling must +// exactly match between all underlying providers of the mux server. Mismatched +// schemas will result in a runtime error. +// see: https://developer.hashicorp.com/terraform/plugin/framework/migrating/mux +// +// Any tests that use this function will serve as a smoketest to verify the +// provider schemas match 1-1 so that we may catch runtime errors. +func protoV5ProviderFactoriesInit(ctx context.Context, providerNames ...string) map[string]func() (tfprotov5.ProviderServer, error) { + factories := make(map[string]func() (tfprotov5.ProviderServer, error), len(providerNames)) + + for _, name := range providerNames { + factories[name] = func() (tfprotov5.ProviderServer, error) { + providerServerFactory, _, err := vault.ProtoV5ProviderServerFactory(ctx) + if err != nil { + return nil, err + } + + return providerServerFactory(), nil + } + } + + return factories +} diff --git a/internal/vault/auth/README.md b/internal/vault/auth/README.md new file mode 100644 index 000000000..114721f65 --- /dev/null +++ b/internal/vault/auth/README.md @@ -0,0 +1,3 @@ +# Vault Auth Methods + +This package contains Vault [auth method API](https://developer.hashicorp.com/vault/api-docs/auth) resources and datasources. diff --git a/internal/vault/secrets/README.md b/internal/vault/secrets/README.md new file mode 100644 index 000000000..071634415 --- /dev/null +++ b/internal/vault/secrets/README.md @@ -0,0 +1,3 @@ +# Vault Secrets Engine + +This package contains Vault [secrets engine API](https://developer.hashicorp.com/vault/api-docs/secret) resources and datasources. diff --git a/internal/vault/sys/README.md b/internal/vault/sys/README.md new file mode 100644 index 000000000..c77c76b6f --- /dev/null +++ b/internal/vault/sys/README.md @@ -0,0 +1,3 @@ +# Vault System Backend + +This package contains Vault [system backend API](https://developer.hashicorp.com/vault/api-docs/system) resources and datasources. diff --git a/internal/sys/password_policy.go b/internal/vault/sys/password_policy.go similarity index 100% rename from internal/sys/password_policy.go rename to internal/vault/sys/password_policy.go diff --git a/internal/vault/sys/password_policy_test.go b/internal/vault/sys/password_policy_test.go new file mode 100644 index 000000000..2664fc63e --- /dev/null +++ b/internal/vault/sys/password_policy_test.go @@ -0,0 +1,48 @@ +// Copyright (c) HashiCorp, Inc. +// SPDX-License-Identifier: MPL-2.0 + +package sys_test + +import ( + "fmt" + "testing" + + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/acctest" + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" + "github.com/hashicorp/terraform-provider-vault/internal/providertest" + "github.com/hashicorp/terraform-provider-vault/testutil" +) + +func TestAccPasswordPolicy(t *testing.T) { + policyName := acctest.RandomWithPrefix("test-policy") + resource.Test(t, resource.TestCase{ + PreCheck: func() { testutil.TestAccPreCheck(t) }, + ProtoV5ProviderFactories: providertest.ProtoV5ProviderFactories, + Steps: []resource.TestStep{ + { + Config: testAccPasswordPolicyConfig(policyName, "length = 20\nrule \"charset\" {\n charset = \"abcde\"\n}\n"), + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttr("vault_password_policy.test", "name", policyName), + resource.TestCheckResourceAttrSet("vault_password_policy.test", "policy"), + ), + }, + { + Config: testAccPasswordPolicyConfig(policyName, "length = 20\nrule \"charset\" {\n charset = \"abcde\"\n}\nrule \"charset\" {\n charset = \"1234567890\"\nmin-chars = 1\n}\n"), + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttr("vault_password_policy.test", "name", policyName), + resource.TestCheckResourceAttrSet("vault_password_policy.test", "policy"), + ), + }, + }, + }) +} + +func testAccPasswordPolicyConfig(policyName string, policy string) string { + return fmt.Sprintf(` +resource "vault_password_policy" "test" { + name = "%s" + policy = < Date: Thu, 14 Dec 2023 15:20:52 -0600 Subject: [PATCH 2/4] Add acc tests for password policy resource --- internal/framework/base/base.go | 12 +++ internal/vault/sys/password_policy.go | 14 +++- internal/vault/sys/password_policy_test.go | 15 ++-- vault/provider.go | 4 - vault/resource_password_policy.go | 52 ------------ vault/resource_password_policy_test.go | 98 ---------------------- 6 files changed, 34 insertions(+), 161 deletions(-) delete mode 100644 vault/resource_password_policy.go delete mode 100644 vault/resource_password_policy_test.go diff --git a/internal/framework/base/base.go b/internal/framework/base/base.go index 6013c54b0..4b3fb7a38 100644 --- a/internal/framework/base/base.go +++ b/internal/framework/base/base.go @@ -13,12 +13,24 @@ import ( ) // BaseModel describes common fields for all of the Terraform resource data models +// +// Ideally this struct would be imbedded into all Resources and DataSources. +// However, the Terraform Plugin Framework doesn't support unmarshalling nested +// structs. See https://github.com/hashicorp/terraform-plugin-framework/issues/242 +// So for now, we must duplicate all fields. type BaseModel struct { + ID types.String `tfsdk:"id"` Namespace types.String `tfsdk:"namespace"` } func baseSchema() map[string]schema.Attribute { return map[string]schema.Attribute{ + // Required for acceptance testing + // https://developer.hashicorp.com/terraform/plugin/framework/acctests#no-id-found-in-attributes + "id": schema.StringAttribute{ + Computed: true, + MarkdownDescription: "ID required by the testing framework", + }, consts.FieldNamespace: schema.StringAttribute{ Optional: true, PlanModifiers: []planmodifier.String{ diff --git a/internal/vault/sys/password_policy.go b/internal/vault/sys/password_policy.go index 8232427ce..c2fd6b7c0 100644 --- a/internal/vault/sys/password_policy.go +++ b/internal/vault/sys/password_policy.go @@ -26,14 +26,17 @@ type PasswordPolicyResource struct { } func (r *PasswordPolicyResource) Metadata(_ context.Context, req resource.MetadataRequest, resp *resource.MetadataResponse) { - resp.TypeName = req.ProviderTypeName + "_password_policy_fw" + resp.TypeName = req.ProviderTypeName + "_password_policy" } // PasswordPolicyModel describes the Terraform resource data model to match the // resource schema. type PasswordPolicyModel struct { + // common fields to all resources + ID types.String `tfsdk:"id"` Namespace types.String `tfsdk:"namespace"` + // fields specific to this resource Name types.String `tfsdk:"name"` Policy types.String `tfsdk:"policy"` } @@ -109,6 +112,9 @@ func (r *PasswordPolicyResource) Create(ctx context.Context, req resource.Create return } + // write the ID to state which is required for acceptance testing + plan.ID = types.StringValue(path) + // Save data into Terraform state resp.Diagnostics.Append(resp.State.Set(ctx, &plan)...) } @@ -177,6 +183,9 @@ func (r *PasswordPolicyResource) Read(ctx context.Context, req resource.ReadRequ state.Policy = types.StringValue(readResp.Policy) + // write the ID to state which is required for acceptance testing + state.ID = types.StringValue(path) + resp.Diagnostics.Append(resp.State.Set(ctx, &state)...) } @@ -213,6 +222,9 @@ func (r *PasswordPolicyResource) Update(ctx context.Context, req resource.Update return } + // write the ID to state which is required for acceptance testing + plan.ID = types.StringValue(path) + // Save data into Terraform state resp.Diagnostics.Append(resp.State.Set(ctx, &plan)...) } diff --git a/internal/vault/sys/password_policy_test.go b/internal/vault/sys/password_policy_test.go index 2664fc63e..302a7ee31 100644 --- a/internal/vault/sys/password_policy_test.go +++ b/internal/vault/sys/password_policy_test.go @@ -15,22 +15,25 @@ import ( func TestAccPasswordPolicy(t *testing.T) { policyName := acctest.RandomWithPrefix("test-policy") + resourceName := "vault_password_policy.test" resource.Test(t, resource.TestCase{ PreCheck: func() { testutil.TestAccPreCheck(t) }, ProtoV5ProviderFactories: providertest.ProtoV5ProviderFactories, Steps: []resource.TestStep{ { - Config: testAccPasswordPolicyConfig(policyName, "length = 20\nrule \"charset\" {\n charset = \"abcde\"\n}\n"), + ProtoV5ProviderFactories: providertest.ProtoV5ProviderFactories, + Config: testAccPasswordPolicyConfig(policyName, "length = 20\nrule \"charset\" {\n charset = \"abcde\"\n}\n"), Check: resource.ComposeTestCheckFunc( - resource.TestCheckResourceAttr("vault_password_policy.test", "name", policyName), - resource.TestCheckResourceAttrSet("vault_password_policy.test", "policy"), + resource.TestCheckResourceAttr(resourceName, "name", policyName), + resource.TestCheckResourceAttrSet(resourceName, "policy"), ), }, { - Config: testAccPasswordPolicyConfig(policyName, "length = 20\nrule \"charset\" {\n charset = \"abcde\"\n}\nrule \"charset\" {\n charset = \"1234567890\"\nmin-chars = 1\n}\n"), + ProtoV5ProviderFactories: providertest.ProtoV5ProviderFactories, + Config: testAccPasswordPolicyConfig(policyName, "length = 20\nrule \"charset\" {\n charset = \"abcde\"\n}\nrule \"charset\" {\n charset = \"1234567890\"\nmin-chars = 1\n}\n"), Check: resource.ComposeTestCheckFunc( - resource.TestCheckResourceAttr("vault_password_policy.test", "name", policyName), - resource.TestCheckResourceAttrSet("vault_password_policy.test", "policy"), + resource.TestCheckResourceAttr(resourceName, "name", policyName), + resource.TestCheckResourceAttrSet(resourceName, "policy"), ), }, }, diff --git a/vault/provider.go b/vault/provider.go index 31843dda9..ef3dde44b 100644 --- a/vault/provider.go +++ b/vault/provider.go @@ -553,10 +553,6 @@ var ( Resource: UpdateSchemaResource(rabbitMQSecretBackendRoleResource()), PathInventory: []string{"/rabbitmq/roles/{name}"}, }, - "vault_password_policy": { - Resource: UpdateSchemaResource(passwordPolicyResource()), - PathInventory: []string{"/sys/policy/password/{name}"}, - }, "vault_pki_secret_backend_cert": { Resource: UpdateSchemaResource(pkiSecretBackendCertResource()), PathInventory: []string{"/pki/issue/{role}"}, diff --git a/vault/resource_password_policy.go b/vault/resource_password_policy.go deleted file mode 100644 index ead6839b1..000000000 --- a/vault/resource_password_policy.go +++ /dev/null @@ -1,52 +0,0 @@ -// Copyright (c) HashiCorp, Inc. -// SPDX-License-Identifier: MPL-2.0 - -package vault - -import ( - "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" - - "github.com/hashicorp/terraform-provider-vault/internal/provider" -) - -var passwordPolicyAttributes = []string{"policy"} - -func passwordPolicyResource() *schema.Resource { - return &schema.Resource{ - Create: resourcePasswordPolicyWrite, - Update: resourcePasswordPolicyWrite, - Delete: resourcePasswordPolicyDelete, - Read: provider.ReadWrapper(resourcePasswordPolicyRead), - - Importer: &schema.ResourceImporter{ - State: schema.ImportStatePassthrough, - }, - - Schema: map[string]*schema.Schema{ - "name": { - Type: schema.TypeString, - Required: true, - ForceNew: true, - Description: "Name of the password policy.", - }, - - "policy": { - Type: schema.TypeString, - Required: true, - Description: "The password policy document", - }, - }, - } -} - -func resourcePasswordPolicyWrite(d *schema.ResourceData, meta interface{}) error { - return passwordPolicyWrite(passwordPolicyAttributes, d, meta) -} - -func resourcePasswordPolicyDelete(d *schema.ResourceData, meta interface{}) error { - return passwordPolicyDelete(d, meta) -} - -func resourcePasswordPolicyRead(d *schema.ResourceData, meta interface{}) error { - return passwordPolicyRead(passwordPolicyAttributes, d, meta) -} diff --git a/vault/resource_password_policy_test.go b/vault/resource_password_policy_test.go deleted file mode 100644 index d1a22c6b3..000000000 --- a/vault/resource_password_policy_test.go +++ /dev/null @@ -1,98 +0,0 @@ -// Copyright (c) HashiCorp, Inc. -// SPDX-License-Identifier: MPL-2.0 - -package vault - -import ( - "fmt" - "testing" - - "github.com/hashicorp/terraform-plugin-sdk/v2/helper/acctest" - "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" - "github.com/hashicorp/terraform-plugin-sdk/v2/terraform" - - "github.com/hashicorp/terraform-provider-vault/internal/provider" - "github.com/hashicorp/terraform-provider-vault/testutil" -) - -func TestAccPasswordPolicy(t *testing.T) { - policyName := acctest.RandomWithPrefix("test-policy") - resource.Test(t, resource.TestCase{ - PreCheck: func() { testutil.TestAccPreCheck(t) }, - ProviderFactories: providerFactories, - CheckDestroy: testAccPasswordPolicyCheckDestroy, - Steps: []resource.TestStep{ - { - Config: testAccPasswordPolicy(policyName, "length = 20\nrule \"charset\" {\n charset = \"abcde\"\n}\n"), - Check: resource.ComposeTestCheckFunc( - resource.TestCheckResourceAttr("vault_password_policy.test", "name", policyName), - resource.TestCheckResourceAttrSet("vault_password_policy.test", "policy"), - ), - }, - { - Config: testAccPasswordPolicy(policyName, "length = 20\nrule \"charset\" {\n charset = \"abcde\"\n}\nrule \"charset\" {\n charset = \"1234567890\"\nmin-chars = 1\n}\n"), - Check: resource.ComposeTestCheckFunc( - resource.TestCheckResourceAttr("vault_password_policy.test", "name", policyName), - resource.TestCheckResourceAttrSet("vault_password_policy.test", "policy"), - ), - }, - }, - }) -} - -func TestAccPasswordPolicy_import(t *testing.T) { - policyName := acctest.RandomWithPrefix("test-policy") - - resource.Test(t, resource.TestCase{ - PreCheck: func() { testutil.TestAccPreCheck(t) }, - ProviderFactories: providerFactories, - CheckDestroy: testAccPasswordPolicyCheckDestroy, - Steps: []resource.TestStep{ - { - Config: testAccPasswordPolicy(policyName, "length = 20\nrule \"charset\" {\n charset = \"abcde\"\n}\n"), - Check: resource.ComposeTestCheckFunc( - resource.TestCheckResourceAttr("vault_password_policy.test", "name", policyName), - resource.TestCheckResourceAttrSet("vault_password_policy.test", "policy"), - ), - }, - { - ResourceName: "vault_password_policy.test", - ImportState: true, - ImportStateVerify: true, - }, - }, - }) -} - -func testAccPasswordPolicyCheckDestroy(s *terraform.State) error { - for _, rs := range s.RootModule().Resources { - if rs.Type != "vault_password_policy" { - continue - } - - client, e := provider.GetClient(rs.Primary, testProvider.Meta()) - if e != nil { - return e - } - - name := rs.Primary.Attributes["name"] - data, err := client.Logical().Read(fmt.Sprintf("sys/policies/password/%s", name)) - if err != nil { - return err - } - if data != nil { - return fmt.Errorf("Password policy %s still exists", name) - } - } - return nil -} - -func testAccPasswordPolicy(policyName string, policy string) string { - return fmt.Sprintf(` -resource "vault_password_policy" "test" { - name = "%s" - policy = < Date: Thu, 14 Dec 2023 15:26:27 -0600 Subject: [PATCH 3/4] remove provider factory from test step --- internal/vault/sys/password_policy_test.go | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/internal/vault/sys/password_policy_test.go b/internal/vault/sys/password_policy_test.go index 302a7ee31..85cb851a7 100644 --- a/internal/vault/sys/password_policy_test.go +++ b/internal/vault/sys/password_policy_test.go @@ -21,16 +21,14 @@ func TestAccPasswordPolicy(t *testing.T) { ProtoV5ProviderFactories: providertest.ProtoV5ProviderFactories, Steps: []resource.TestStep{ { - ProtoV5ProviderFactories: providertest.ProtoV5ProviderFactories, - Config: testAccPasswordPolicyConfig(policyName, "length = 20\nrule \"charset\" {\n charset = \"abcde\"\n}\n"), + Config: testAccPasswordPolicyConfig(policyName, "length = 20\nrule \"charset\" {\n charset = \"abcde\"\n}\n"), Check: resource.ComposeTestCheckFunc( resource.TestCheckResourceAttr(resourceName, "name", policyName), resource.TestCheckResourceAttrSet(resourceName, "policy"), ), }, { - ProtoV5ProviderFactories: providertest.ProtoV5ProviderFactories, - Config: testAccPasswordPolicyConfig(policyName, "length = 20\nrule \"charset\" {\n charset = \"abcde\"\n}\nrule \"charset\" {\n charset = \"1234567890\"\nmin-chars = 1\n}\n"), + Config: testAccPasswordPolicyConfig(policyName, "length = 20\nrule \"charset\" {\n charset = \"abcde\"\n}\nrule \"charset\" {\n charset = \"1234567890\"\nmin-chars = 1\n}\n"), Check: resource.ComposeTestCheckFunc( resource.TestCheckResourceAttr(resourceName, "name", policyName), resource.TestCheckResourceAttrSet(resourceName, "policy"), From 17f652a065efbd83fdda4221dd2bc5301a2b6fe1 Mon Sep 17 00:00:00 2001 From: JM Faircloth Date: Mon, 18 Dec 2023 16:00:24 -0600 Subject: [PATCH 4/4] remove password_policy.go --- vault/password_policy.go | 122 --------------------------------------- 1 file changed, 122 deletions(-) delete mode 100644 vault/password_policy.go diff --git a/vault/password_policy.go b/vault/password_policy.go deleted file mode 100644 index b42612d60..000000000 --- a/vault/password_policy.go +++ /dev/null @@ -1,122 +0,0 @@ -// Copyright (c) HashiCorp, Inc. -// SPDX-License-Identifier: MPL-2.0 - -package vault - -import ( - "context" - "errors" - "fmt" - "log" - - "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" - "github.com/hashicorp/vault/api" - - "github.com/hashicorp/terraform-provider-vault/internal/provider" -) - -func readPasswordPolicy(client *api.Client, name string) (map[string]interface{}, error) { - r := client.NewRequest("GET", fmt.Sprintf("/v1/sys/policies/password/%s", name)) - - ctx, cancelFunc := context.WithCancel(context.Background()) - defer cancelFunc() - resp, err := client.RawRequestWithContext(ctx, r) - if resp != nil { - defer resp.Body.Close() - if resp.StatusCode == 404 { - return nil, nil - } - } - if err != nil { - return nil, err - } - - secret, err := api.ParseSecret(resp.Body) - if err != nil { - return nil, err - } - if secret == nil || secret.Data == nil { - return nil, errors.New("data from server response is empty") - } - return secret.Data, nil -} - -func passwordPolicyDelete(d *schema.ResourceData, meta interface{}) error { - client, e := provider.GetClient(d, meta) - if e != nil { - return e - } - - name := d.Id() - - log.Printf("[DEBUG] Deleting %s password policy from Vault", name) - - r := client.NewRequest("DELETE", fmt.Sprintf("/v1/sys/policies/password/%s", name)) - - ctx, cancelFunc := context.WithCancel(context.Background()) - defer cancelFunc() - resp, err := client.RawRequestWithContext(ctx, r) - if err == nil { - defer resp.Body.Close() - } - - return err -} - -func passwordPolicyRead(attributes []string, d *schema.ResourceData, meta interface{}) error { - client, e := provider.GetClient(d, meta) - if e != nil { - return e - } - - name := d.Id() - - policy, err := readPasswordPolicy(client, name) - if err != nil { - return fmt.Errorf("error reading from Vault: %s", err) - } - - for _, value := range attributes { - d.Set(value, policy[value]) - } - d.Set("name", name) - - return nil -} - -func passwordPolicyWrite(attributes []string, d *schema.ResourceData, meta interface{}) error { - client, e := provider.GetClient(d, meta) - if e != nil { - return e - } - - name := d.Get("name").(string) - - log.Printf("[DEBUG] Writing %s password policy to Vault", name) - - body := map[string]interface{}{} - for _, value := range attributes { - body[value] = d.Get(value) - } - - r := client.NewRequest("PUT", fmt.Sprintf("/v1/sys/policies/password/%s", name)) - if err := r.SetJSONBody(body); err != nil { - return err - } - - ctx, cancelFunc := context.WithCancel(context.Background()) - defer cancelFunc() - resp, err := client.RawRequestWithContext(ctx, r) - if err != nil { - return err - } - defer resp.Body.Close() - - if err != nil { - return fmt.Errorf("error writing to Vault: %s", err) - } - - d.SetId(name) - - return passwordPolicyRead(attributes, d, meta) -}