diff --git a/spinifex/handlers/acm/service_impl.go b/spinifex/handlers/acm/service_impl.go index 5467cc422..9c9fba056 100644 --- a/spinifex/handlers/acm/service_impl.go +++ b/spinifex/handlers/acm/service_impl.go @@ -210,10 +210,16 @@ func (s *ACMServiceImpl) ImportCertificate(ctx context.Context, input *acm.Impor certArn := aws.StringValue(input.CertificateArn) var inUseBy []string + tags := tagsToMap(input.Tags) reimport := certArn != "" if certArn == "" { certArn = s.mintCertificateArn(accountID) } else { + // AWS refuses tags on re-import. The stored tags carry forward below, + // so a plain re-import does not clear them. + if len(input.Tags) > 0 { + return nil, awserrors.Errorf(awserrors.ErrorValidationException, "Tagging is not permitted on re-import.") + } // Re-import: the ARN must be one the gate can read, and must already // exist and belong to the caller. if _, ok := arn.ParseACMCertificateID(certArn); !ok { @@ -229,6 +235,7 @@ func (s *ACMServiceImpl) ImportCertificate(ctx context.Context, input *acm.Impor // Carry the InUseBy index forward — new material under the same ARN // must not silently drop the load balancers that reference it. inUseBy = existing.InUseBy + tags = existing.Tags } rec := &CertRecord{ @@ -246,7 +253,7 @@ func (s *ACMServiceImpl) ImportCertificate(ctx context.Context, input *acm.Impor NotBefore: leaf.NotBefore, NotAfter: leaf.NotAfter, ImportedAt: time.Now().UTC(), - Tags: tagsToMap(input.Tags), + Tags: tags, InUseBy: inUseBy, Type: certTypeImported, Status: certStatusIssued, diff --git a/spinifex/handlers/acm/tags_test.go b/spinifex/handlers/acm/tags_test.go index b9b55fd34..eea09acfd 100644 --- a/spinifex/handlers/acm/tags_test.go +++ b/spinifex/handlers/acm/tags_test.go @@ -108,3 +108,69 @@ func TestTagOps_CrossAccountHidden(t *testing.T) { }, "000000000002") assert.Equal(t, awserrors.ErrorResourceNotFound, err.Error()) } + +func listTagMap(t *testing.T, svc *ACMServiceImpl, arn string) map[string]string { + t.Helper() + out, err := svc.ListTagsForCertificate(context.Background(), &acm.ListTagsForCertificateInput{ + CertificateArn: aws.String(arn), + }, testAccountID) + require.NoError(t, err) + got := map[string]string{} + for _, tg := range out.Tags { + got[aws.StringValue(tg.Key)] = aws.StringValue(tg.Value) + } + return got +} + +// AWS answers a re-import carrying Tags with ValidationException and leaves +// the stored certificate, material and tags alike, untouched. +func TestImportCertificate_ReimportWithTagsRejected(t *testing.T) { + svc := setupACMService(t) + arn := importTagged(t, svc, &acm.Tag{Key: aws.String("Name"), Value: aws.String("ingress")}) + + c2, k2 := genCert(t, "rotated.example.com", "rotated.example.com") + _, err := svc.ImportCertificate(context.Background(), &acm.ImportCertificateInput{ + Certificate: c2, + PrivateKey: k2, + CertificateArn: aws.String(arn), + Tags: []*acm.Tag{{Key: aws.String("env"), Value: aws.String("prod")}}, + }, testAccountID) + require.Error(t, err) + + code, message, ok := awserrors.ResolveErrorDetail(err) + require.True(t, ok) + assert.Equal(t, awserrors.ErrorValidationException, code) + assert.Equal(t, "Tagging is not permitted on re-import.", message) + assert.Equal(t, 400, awserrors.ErrorLookup[code].HTTPCode) + + desc, err := svc.DescribeCertificate(context.Background(), &acm.DescribeCertificateInput{CertificateArn: aws.String(arn)}, testAccountID) + require.NoError(t, err) + assert.Equal(t, "tags.example.com", aws.StringValue(desc.Certificate.DomainName), "a refused re-import must not replace the material") + assert.Equal(t, map[string]string{"Name": "ingress"}, listTagMap(t, svc, arn)) +} + +// A re-import without Tags keeps the tags already on the certificate rather +// than clearing them, as InUseBy is kept. +func TestImportCertificate_ReimportKeepsStoredTags(t *testing.T) { + svc := setupACMService(t) + arn := importTagged(t, svc, &acm.Tag{Key: aws.String("Name"), Value: aws.String("ingress")}) + _, err := svc.AddTagsToCertificate(context.Background(), &acm.AddTagsToCertificateInput{ + CertificateArn: aws.String(arn), + Tags: []*acm.Tag{{Key: aws.String("env"), Value: aws.String("dev")}}, + }, testAccountID) + require.NoError(t, err) + + c2, k2 := genCert(t, "rotated.example.com", "rotated.example.com") + out, err := svc.ImportCertificate(context.Background(), &acm.ImportCertificateInput{ + Certificate: c2, + PrivateKey: k2, + CertificateArn: aws.String(arn), + }, testAccountID) + require.NoError(t, err) + assert.Equal(t, arn, aws.StringValue(out.CertificateArn)) + + desc, err := svc.DescribeCertificate(context.Background(), &acm.DescribeCertificateInput{CertificateArn: aws.String(arn)}, testAccountID) + require.NoError(t, err) + assert.Equal(t, "rotated.example.com", aws.StringValue(desc.Certificate.DomainName), "material must be replaced") + assert.Equal(t, map[string]string{"Name": "ingress", "env": "dev"}, listTagMap(t, svc, arn)) +}