fix: reject Tags on ACM certificate re-import and keep the stored tags (#1177)

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,

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))
+}