Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

#13605 added base64decode string before sending to aws-sdk-go, so cer… #17958

Merged
merged 2 commits into from
Mar 17, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .changelog/17958.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:bug
resource/aws_dms_certificate: Correctly base64 decode `certificate_wallet` value
```
71 changes: 50 additions & 21 deletions aws/resource_aws_dms_certificate.go
Original file line number Diff line number Diff line change
@@ -1,13 +1,16 @@
package aws

import (
"encoding/base64"
"fmt"
"log"
"regexp"

"github.com/aws/aws-sdk-go/aws"
"github.com/aws/aws-sdk-go/aws/awserr"
dms "github.com/aws/aws-sdk-go/service/databasemigrationservice"
"github.com/hashicorp/aws-sdk-go-base/tfawserr"
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/validation"
"github.com/terraform-providers/terraform-provider-aws/aws/internal/keyvaluetags"
)

Expand All @@ -28,10 +31,15 @@ func resourceAwsDmsCertificate() *schema.Resource {
Computed: true,
},
"certificate_id": {
Type: schema.TypeString,
Required: true,
ForceNew: true,
ValidateFunc: validateDmsCertificateId,
Type: schema.TypeString,
Required: true,
ForceNew: true,
ValidateFunc: validation.All(
validation.StringLenBetween(1, 255),
validation.StringMatch(regexp.MustCompile("^[a-zA-Z][a-zA-Z0-9-]+$"), "must start with a letter, only contain alphanumeric characters and hyphens"),
validation.StringDoesNotMatch(regexp.MustCompile(`--`), "cannot contain two consecutive hyphens"),
validation.StringDoesNotMatch(regexp.MustCompile(`-$`), "cannot end in a hyphen"),
),
},
"certificate_pem": {
Type: schema.TypeString,
Expand All @@ -52,37 +60,40 @@ func resourceAwsDmsCertificate() *schema.Resource {

func resourceAwsDmsCertificateCreate(d *schema.ResourceData, meta interface{}) error {
conn := meta.(*AWSClient).dmsconn
certificateID := d.Get("certificate_id").(string)

request := &dms.ImportCertificateInput{
CertificateIdentifier: aws.String(d.Get("certificate_id").(string)),
CertificateIdentifier: aws.String(certificateID),
Tags: keyvaluetags.New(d.Get("tags").(map[string]interface{})).IgnoreAws().DatabasemigrationserviceTags(),
}

pem, pemSet := d.GetOk("certificate_pem")
wallet, walletSet := d.GetOk("certificate_wallet")

if !pemSet && !walletSet {
return fmt.Errorf("Must set either certificate_pem and certificate_wallet.")
return fmt.Errorf("Must set either certificate_pem or certificate_wallet for DMS Certificate (%s)", certificateID)
}
if pemSet && walletSet {
return fmt.Errorf("Cannot set both certificate_pem and certificate_wallet.")
return fmt.Errorf("Cannot set both certificate_pem and certificate_wallet for DMS Certificate (%s)", certificateID)
}

if pemSet {
request.CertificatePem = aws.String(pem.(string))
}
if walletSet {
request.CertificateWallet = []byte(wallet.(string))
certWallet, err := base64.StdEncoding.DecodeString(wallet.(string))
if err != nil {
return fmt.Errorf("error Base64 decoding certificate_wallet for DMS Certificate (%s): %w", certificateID, err)
}
request.CertificateWallet = certWallet
}

log.Println("[DEBUG] DMS import certificate:", request)

_, err := conn.ImportCertificate(request)
if err != nil {
return err
return fmt.Errorf("error creating DMS certificate (%s): %w", certificateID, err)
}

d.SetId(d.Get("certificate_id").(string))
d.SetId(certificateID)
return resourceAwsDmsCertificateRead(d, meta)
}

Expand All @@ -98,12 +109,24 @@ func resourceAwsDmsCertificateRead(d *schema.ResourceData, meta interface{}) err
},
},
})

if !d.IsNewResource() && tfawserr.ErrCodeEquals(err, dms.ErrCodeResourceNotFoundFault) {
log.Printf("[WARN] DMS Certificate (%s) not found, removing from state", d.Id())
d.SetId("")
return nil
}

if err != nil {
if dmserr, ok := err.(awserr.Error); ok && dmserr.Code() == "ResourceNotFoundFault" {
d.SetId("")
return nil
return fmt.Errorf("error reading DMS Certificate (%s): %w", d.Id(), err)
}

if response == nil || len(response.Certificates) == 0 || response.Certificates[0] == nil {
if d.IsNewResource() {
return fmt.Errorf("error reading DMS Certificate (%s): not found", d.Id())
}
return err
log.Printf("[WARN] DMS Certificate (%s) not found, removing from state", d.Id())
d.SetId("")
return nil
}

err = resourceAwsDmsCertificateSetState(d, response.Certificates[0])
Expand Down Expand Up @@ -146,10 +169,16 @@ func resourceAwsDmsCertificateDelete(d *schema.ResourceData, meta interface{}) e
CertificateArn: aws.String(d.Get("certificate_arn").(string)),
}

log.Printf("[DEBUG] DMS delete certificate: %#v", request)

_, err := conn.DeleteCertificate(request)
return err

if err != nil {
if tfawserr.ErrCodeEquals(err, dms.ErrCodeResourceNotFoundFault) {
return nil
}
return fmt.Errorf("error deleting DMS Certificate (%s): %w", d.Id(), err)
}

return nil
}

func resourceAwsDmsCertificateSetState(d *schema.ResourceData, cert *dms.Certificate) error {
Expand All @@ -162,7 +191,7 @@ func resourceAwsDmsCertificateSetState(d *schema.ResourceData, cert *dms.Certifi
d.Set("certificate_pem", cert.CertificatePem)
}
if cert.CertificateWallet != nil && len(cert.CertificateWallet) != 0 {
d.Set("certificate_wallet", string(cert.CertificateWallet))
d.Set("certificate_wallet", base64Encode(cert.CertificateWallet))
}

return nil
Expand Down
142 changes: 104 additions & 38 deletions aws/resource_aws_dms_certificate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,9 @@ import (

"github.com/aws/aws-sdk-go/aws"
dms "github.com/aws/aws-sdk-go/service/databasemigrationservice"
"github.com/hashicorp/aws-sdk-go-base/tfawserr"
"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/helper/schema"
"github.com/hashicorp/terraform-plugin-sdk/v2/terraform"
)

Expand All @@ -19,12 +19,12 @@ func TestAccAWSDmsCertificate_basic(t *testing.T) {
resource.ParallelTest(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
Providers: testAccProviders,
CheckDestroy: dmsCertificateDestroy,
CheckDestroy: testAccCheckAWSDmsCertificateDestroy,
Steps: []resource.TestStep{
{
Config: dmsCertificateConfig(randId),
Config: testAccAWSDmsCertificateConfig(randId),
Check: resource.ComposeTestCheckFunc(
checkDmsCertificateExists(resourceName),
testAccAWSDmsCertificateExists(resourceName),
resource.TestCheckResourceAttrSet(resourceName, "certificate_arn"),
),
},
Expand All @@ -37,19 +37,65 @@ func TestAccAWSDmsCertificate_basic(t *testing.T) {
})
}

func TestAccAWSDmsCertificate_disappears(t *testing.T) {
resourceName := "aws_dms_certificate.dms_certificate"
randId := acctest.RandString(8)

resource.ParallelTest(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
Providers: testAccProviders,
CheckDestroy: nil,
Steps: []resource.TestStep{
{
Config: testAccAWSDmsCertificateConfig(randId),
Check: resource.ComposeTestCheckFunc(
testAccAWSDmsCertificateExists(resourceName),
testAccCheckResourceDisappears(testAccProvider, resourceAwsDmsCertificate(), resourceName),
),
ExpectNonEmptyPlan: true,
},
},
})
}

func TestAccAWSDmsCertificate_CertificateWallet(t *testing.T) {
resourceName := "aws_dms_certificate.dms_certificate"
rName := acctest.RandomWithPrefix("tf-acc-test")

resource.ParallelTest(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
Providers: testAccProviders,
CheckDestroy: testAccCheckAWSDmsCertificateDestroy,
Steps: []resource.TestStep{
{
Config: testAccAWSDmsCertificateConfigCertificateWallet(rName),
Check: resource.ComposeTestCheckFunc(
testAccAWSDmsCertificateExists(resourceName),
resource.TestCheckResourceAttrSet(resourceName, "certificate_wallet"),
),
},
{
ResourceName: resourceName,
ImportState: true,
ImportStateVerify: true,
},
},
})
}

func TestAccAWSDmsCertificate_tags(t *testing.T) {
resourceName := "aws_dms_certificate.dms_certificate"
randId := acctest.RandString(8)

resource.ParallelTest(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
Providers: testAccProviders,
CheckDestroy: dmsCertificateDestroy,
CheckDestroy: testAccCheckAWSDmsCertificateDestroy,
Steps: []resource.TestStep{
{
Config: testAccAWSDmsCertificateConfigTags1(randId, "key1", "value1"),
Check: resource.ComposeAggregateTestCheckFunc(
checkDmsCertificateExists(resourceName),
testAccAWSDmsCertificateExists(resourceName),
resource.TestCheckResourceAttr(resourceName, "tags.%", "1"),
resource.TestCheckResourceAttr(resourceName, "tags.key1", "value1"),
),
Expand All @@ -62,7 +108,7 @@ func TestAccAWSDmsCertificate_tags(t *testing.T) {
{
Config: testAccAWSDmsCertificateConfigTags2(randId, "key1", "value1updated", "key2", "value2"),
Check: resource.ComposeAggregateTestCheckFunc(
checkDmsCertificateExists(resourceName),
testAccAWSDmsCertificateExists(resourceName),
resource.TestCheckResourceAttr(resourceName, "tags.%", "2"),
resource.TestCheckResourceAttr(resourceName, "tags.key1", "value1updated"),
resource.TestCheckResourceAttr(resourceName, "tags.key2", "value2"),
Expand All @@ -71,7 +117,7 @@ func TestAccAWSDmsCertificate_tags(t *testing.T) {
{
Config: testAccAWSDmsCertificateConfigTags1(randId, "key2", "value2"),
Check: resource.ComposeAggregateTestCheckFunc(
checkDmsCertificateExists(resourceName),
testAccAWSDmsCertificateExists(resourceName),
resource.TestCheckResourceAttr(resourceName, "tags.%", "1"),
resource.TestCheckResourceAttr(resourceName, "tags.key2", "value2"),
),
Expand All @@ -80,27 +126,40 @@ func TestAccAWSDmsCertificate_tags(t *testing.T) {
})
}

func dmsCertificateDestroy(s *terraform.State) error {
func testAccCheckAWSDmsCertificateDestroy(s *terraform.State) error {
for _, rs := range s.RootModule().Resources {
if rs.Type != "aws_dms_certificate" {
continue
}

err := checkDmsCertificateExists(rs.Primary.ID)
if err == nil {
return fmt.Errorf("Found a certificate that was not destroyed: %s", rs.Primary.ID)
conn := testAccProvider.Meta().(*AWSClient).dmsconn

output, err := conn.DescribeCertificates(&dms.DescribeCertificatesInput{
Filters: []*dms.Filter{
{
Name: aws.String("certificate-id"),
Values: []*string{aws.String(rs.Primary.ID)},
},
},
})

if tfawserr.ErrCodeEquals(err, dms.ErrCodeResourceNotFoundFault) {
continue
}

if err != nil {
return fmt.Errorf("error reading DMS Certificate (%s): %w", rs.Primary.ID, err)
}

if output != nil && len(output.Certificates) != 0 {
return fmt.Errorf("DMS Certificate (%s) still exists", rs.Primary.ID)
}
}

return nil
}

func checkDmsCertificateExists(n string) resource.TestCheckFunc {
providers := []*schema.Provider{testAccProvider}
return checkDmsCertificateExistsWithProviders(n, &providers)
}

func checkDmsCertificateExistsWithProviders(n string, providers *[]*schema.Provider) resource.TestCheckFunc {
func testAccAWSDmsCertificateExists(n string) resource.TestCheckFunc {
return func(s *terraform.State) error {
rs, ok := s.RootModule().Resources[n]
if !ok {
Expand All @@ -110,33 +169,31 @@ func checkDmsCertificateExistsWithProviders(n string, providers *[]*schema.Provi
if rs.Primary.ID == "" {
return fmt.Errorf("No ID is set")
}
for _, provider := range *providers {
// Ignore if Meta is empty, this can happen for validation providers
if provider.Meta() == nil {
continue
}

conn := provider.Meta().(*AWSClient).dmsconn
_, err := conn.DescribeCertificates(&dms.DescribeCertificatesInput{
Filters: []*dms.Filter{
{
Name: aws.String("certificate-id"),
Values: []*string{aws.String(rs.Primary.ID)},
},

conn := testAccProvider.Meta().(*AWSClient).dmsconn

output, err := conn.DescribeCertificates(&dms.DescribeCertificatesInput{
Filters: []*dms.Filter{
{
Name: aws.String("certificate-id"),
Values: []*string{aws.String(rs.Primary.ID)},
},
})
},
})

if err != nil {
return fmt.Errorf("DMS certificate error: %v", err)
}
return nil
if err != nil {
return fmt.Errorf("error reading DMS Certificate (%s): %w", rs.Primary.ID, err)
}

return fmt.Errorf("DMS certificate not found")
if output == nil || len(output.Certificates) == 0 {
return fmt.Errorf("DMS Certificate (%s) not found", rs.Primary.ID)
}

return nil
}
}

func dmsCertificateConfig(randId string) string {
func testAccAWSDmsCertificateConfig(randId string) string {
return fmt.Sprintf(`
resource "aws_dms_certificate" "dms_certificate" {
certificate_id = "tf-test-dms-certificate-%[1]s"
Expand All @@ -145,6 +202,15 @@ resource "aws_dms_certificate" "dms_certificate" {
`, randId)
}

func testAccAWSDmsCertificateConfigCertificateWallet(rName string) string {
return fmt.Sprintf(`
resource "aws_dms_certificate" "dms_certificate" {
certificate_id = %q
certificate_wallet = filebase64("testdata/service/dms/oracle_wallet_certificate.pem")
}
`, rName)
}

func testAccAWSDmsCertificateConfigTags1(randId, tagKey1, tagValue1 string) string {
return fmt.Sprintf(`
resource "aws_dms_certificate" "dms_certificate" {
Expand Down
10 changes: 10 additions & 0 deletions aws/testdata/service/dms/oracle_wallet_certificate.pem
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
-----BEGIN CERTIFICATE-----
MIIBsTCCARoCAQAwDQYJKoZIhvcNAQEEBQAwITEfMB0GA1UEAxMWZ2JyMzAxMzkudWsub3JhY2xl
LmNvbTAeFw0xNjA1MTExMTQzMzFaFw0yNjA1MDkxMTQzMzFaMCExHzAdBgNVBAMTFmdicjMwMTM5
LnVrLm9yYWNsZS5jb20wgZ8wDQYJKoZIhvcNAQEBBQADgY0AMIGJAoGBAKH8G8sFS6l0llu+RMfl
7Yt+Ppw8J0PfDEDbTGP5wtsrs/22dUCipU9l+vif1VgSPLE2UPJbGM8tQzTC6UYbBtWHe4CshmvD
EVlcIMsEFvD7a5Q+P45jqNSEtV9VdbGyxaD6i5Y/Smd+B87FcQQCX54LaI9BJ8SZwmPXgDweADLf
AgMBAAEwDQYJKoZIhvcNAQEEBQADgYEAai742jfNYYTKMq2xxRygGJGn1LhpFenHvuHLBvnTup1N
nZOBwBi4VxW3CImvwONYcCEFp3E1SRswS5evlfIfruCZ1xQBoUNei3EJ6O3OdKeRRp2E+muXEtfe
U+jwUE+SzpnzfpI23Okl2vo8Q7VHrSalxE2KEhAzC1UYX7ZYp1U=
-----END CERTIFICATE-----
Loading