chore: Adds publish transformers and validation (#2585)

Adds publish validator for name/workers.
Adds publish converters.

Addresses #2550

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

Author: xar-tol

internal/pkg/deploy/cloudformation/stack/transformers.go

@@ -581,3 +581,38 @@ func convertCommand(command *manifest.CommandOverride) ([]string, error) {
 	}
 	return out, nil
 }
+
+func convertPublish(p *manifest.PublishConfig) (*template.PublishOpts, error) {
+	if p == nil || len(p.Topics) == 0 {
+		return nil, nil
+	}
+	publishers := template.PublishOpts{}
+	// convert the topics to template Topics
+	for _, topic := range p.Topics {
+		t, err := convertTopic(topic)
+		if err != nil {
+			return nil, err
+		}
+
+		publishers.Topics = append(publishers.Topics, t)
+	}
+
+	return &publishers, nil
+}
+
+func convertTopic(t manifest.Topic) (*template.Topics, error) {
+	err := validatePubSubName(t.Name)
+	if err != nil {
+		return nil, err
+	}
+
+	workerErr := validateWorkerNames(t.AllowedWorkers)
+	if workerErr != nil {
+		return nil, workerErr
+	}
+
+	return &template.Topics{
+		Name:           t.Name,
+		AllowedWorkers: t.AllowedWorkers,
+	}, nil
+}

internal/pkg/deploy/cloudformation/stack/transformers_test.go

@@ -1441,3 +1441,91 @@ func Test_convertImageDependsOn(t *testing.T) {
 		})
 	}
 }
+
+func Test_convertPublish(t *testing.T) {
+	testCases := map[string]struct {
+		inPublish *manifest.PublishConfig
+
+		wanted      *template.PublishOpts
+		wantedError error
+	}{
+		"empty publish": {
+			inPublish: &manifest.PublishConfig{},
+			wanted:    nil,
+		},
+		"publish with no topic names": {
+			inPublish: &manifest.PublishConfig{
+				Topics: []manifest.Topic{
+					{},
+				},
+			},
+			wantedError: errMissingPublishTopicField,
+		},
+		"publish with no workers": {
+			inPublish: &manifest.PublishConfig{
+				Topics: []manifest.Topic{
+					{
+						Name: aws.String("topic1"),
+					},
+				},
+			},
+			wanted: &template.PublishOpts{
+				Topics: []*template.Topics{
+					{
+						Name: aws.String("topic1"),
+					},
+				},
+			},
+		},
+		"publish with workers": {
+			inPublish: &manifest.PublishConfig{
+				Topics: []manifest.Topic{
+					{
+						Name:           aws.String("topic1"),
+						AllowedWorkers: []string{"worker1"},
+					},
+				},
+			},
+			wanted: &template.PublishOpts{
+				Topics: []*template.Topics{
+					{
+						Name:           aws.String("topic1"),
+						AllowedWorkers: []string{"worker1"},
+					},
+				},
+			},
+		},
+		"invalid worker name": {
+			inPublish: &manifest.PublishConfig{
+				Topics: []manifest.Topic{
+					{
+						Name:           aws.String("topic1"),
+						AllowedWorkers: []string{"worker1~~@#$"},
+					},
+				},
+			},
+			wantedError: fmt.Errorf("worker name `worker1~~@#$` is invalid: %s", errNameBadFormat),
+		},
+		"invalid topic name": {
+			inPublish: &manifest.PublishConfig{
+				Topics: []manifest.Topic{
+					{
+						Name:           aws.String("topic1~~@#$"),
+						AllowedWorkers: []string{"worker1"},
+					},
+				},
+			},
+			wantedError: errInvalidPubSubTopicName,
+		},
+	}
+	for name, tc := range testCases {
+		t.Run(name, func(t *testing.T) {
+			got, err := convertPublish(tc.inPublish)
+			if tc.wantedError != nil {
+				require.EqualError(t, err, tc.wantedError.Error())
+			} else {
+				require.Equal(t, got, tc.wanted)
+			}
+		})
+	}
+}

internal/pkg/deploy/cloudformation/stack/validate.go

@@ -6,6 +6,7 @@ package stack
 import (
 	"errors"
 	"fmt"
+	"regexp"
 
 	"github.com/aws/aws-sdk-go/aws"
 	"github.com/aws/copilot-cli/internal/pkg/manifest"
@@ -21,10 +22,11 @@ const (
 
 // Empty field errors.
 var (
-	errNoFSID          = errors.New("volume field `efs.id` cannot be empty")
-	errNoContainerPath = errors.New("`path` cannot be empty")
-	errNoSourceVolume  = errors.New("`source_volume` cannot be empty")
-	errEmptyEFSConfig  = errors.New("bad EFS configuration: `efs` cannot be empty")
+	errNoFSID                   = errors.New("volume field `efs.id` cannot be empty")
+	errNoContainerPath          = errors.New("`path` cannot be empty")
+	errNoSourceVolume           = errors.New("`source_volume` cannot be empty")
+	errEmptyEFSConfig           = errors.New("bad EFS configuration: `efs` cannot be empty")
+	errMissingPublishTopicField = errors.New("topic `name` cannot be empty")
 )
 
 // Conditional errors.
@@ -40,6 +42,10 @@ var (
 	errInvalidSidecarDependsOnStatus = fmt.Errorf("sidecar container dependency status must be one of < %s | %s | %s >", dependsOnStart, dependsOnComplete, dependsOnSuccess)
 	errEssentialContainerStatus      = fmt.Errorf("essential container dependencies can only have status < %s | %s >", dependsOnStart, dependsOnHealthy)
 	errEssentialSidecarStatus        = fmt.Errorf("essential sidecar container dependencies can only have status < %s >", dependsOnStart)
+	errInvalidPubSubTopicName        = errors.New("topic names can only contain letters, numbers, underscores, and hypthens")
+	errInvalidName                   = errors.New("names cannot be empty")
+	errNameTooLong                   = errors.New("names must not exceed 255 characters")
+	errNameBadFormat                 = errors.New("names must start with a letter, contain only lower-case letters, numbers, and hyphens, and have no consecutive or trailing hyphen")
 )
 
 // Container dependency status options
@@ -49,6 +55,14 @@ var (
 	sidecarDependsOnValidStatuses   = []string{dependsOnStart, dependsOnComplete, dependsOnSuccess}
 )
 
+// Regex options
+var (
+	awsSNSTopicRegexp   = regexp.MustCompile(`^[a-zA-Z0-9_-]*$`)   // Validates that an expression contains only letters, numbers, underscores, and hyphens.
+	awsNameRegexp       = regexp.MustCompile(`^[a-z][a-z0-9\-]+$`) // Validates that an expression starts with a letter and only contains letters, numbers, and hyphens.
+	punctuationRegExp   = regexp.MustCompile(`[\.\-]{2,}`)         // Check for consecutive periods or dashes.
+	trailingPunctRegExp = regexp.MustCompile(`[\-\.]$`)            // Check for trailing dash or dot.
+)
+
 // Validate that paths contain only an approved set of characters to guard against command injection.
 // We can accept 0-9A-Za-z-_.
 func validatePath(input string, maxLength int) error {
@@ -392,3 +406,57 @@ func validateRootDirPath(input string) error {
 func validateContainerPath(input string) error {
 	return validatePath(input, maxDockerContainerPathLength)
 }
+
+// ValidatePubSubName validates naming is correct for topics in publishing/subscribing cases, such as naming for a
+// SNS Topic intended for a publisher.
+func validatePubSubName(name *string) error {
+	if name == nil || len(aws.StringValue(name)) == 0 {
+		return errMissingPublishTopicField
+	}
+
+	// Name must contain letters, numbers, and can't use special characters besides underscores and hyphens.
+	if !awsSNSTopicRegexp.MatchString(aws.StringValue(name)) {
+		return errInvalidPubSubTopicName
+	}
+
+	return nil
+}
+
+func validateWorkerNames(names []string) error {
+	for _, name := range names {
+		err := validateSvcName(name)
+		if err != nil {
+			return fmt.Errorf("worker name `%s` is invalid: %w", name, err)
+		}
+	}
+	return nil
+}
+
+func validateSvcName(name string) error {
+	if name == "" {
+		return errInvalidName
+	}
+	if len(name) > 255 {
+		return errNameTooLong
+	}
+	if !isCorrectSvcNameFormat(name) {
+		return errNameBadFormat
+	}
+
+	return nil
+}
+
+func isCorrectSvcNameFormat(s string) bool {
+	if !awsNameRegexp.MatchString(s) {
+		return false
+	}
+
+	// Check for bad punctuation (no consecutive dashes or dots)
+	formatMatch := punctuationRegExp.FindStringSubmatch(s)
+	if len(formatMatch) != 0 {
+		return false
+	}
+
+	trailingMatch := trailingPunctRegExp.FindStringSubmatch(s)
+	return len(trailingMatch) == 0
+}

internal/pkg/deploy/cloudformation/stack/validate_test.go

@@ -498,3 +498,70 @@ func TestValidateImageDependsOn(t *testing.T) {
 		})
 	}
 }
+
+func Test_validatePubSubTopicName(t *testing.T) {
+	testCases := map[string]struct {
+		inName *string
+
+		wantErr error
+	}{
+		"valid topic name": {
+			inName: aws.String("a-Perfectly_V4l1dString"),
+		},
+		"error when no topic name": {
+			inName:  nil,
+			wantErr: errMissingPublishTopicField,
+		},
+		"error when invalid topic name": {
+			inName:  aws.String("OHNO~/`...,"),
+			wantErr: errInvalidPubSubTopicName,
+		},
+	}
+	for name, tc := range testCases {
+		t.Run(name, func(t *testing.T) {
+			err := validatePubSubName(tc.inName)
+			if tc.wantErr == nil {
+				require.NoError(t, err)
+			} else {
+				require.EqualError(t, err, tc.wantErr.Error())
+			}
+		})
+	}
+}
+
+func TestValidateWorkerName(t *testing.T) {
+	testCases := map[string]struct {
+		inName []string
+
+		wantErr error
+	}{
+		"good case": {
+			inName:  []string{"good-name"},
+			wantErr: nil,
+		},
+		"empty name": {
+			inName:  []string{""},
+			wantErr: fmt.Errorf("worker name `` is invalid: %s", errInvalidName),
+		},
+		"contains spaces": {
+			inName:  []string{"a re@!!y b#d n&me"},
+			wantErr: fmt.Errorf("worker name `a re@!!y b#d n&me` is invalid: %s", errNameBadFormat),
+		},
+		"too long": {
+			inName:  []string{"this-is-the-name-that-goes-on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-until-it-is-too-long"},
+			wantErr: fmt.Errorf("worker name `this-is-the-name-that-goes-on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-until-it-is-too-long` is invalid: %s", errNameTooLong),
+		},
+	}
+
+	for name, tc := range testCases {
+		t.Run(name, func(t *testing.T) {
+			err := validateWorkerNames(tc.inName)
+
+			if tc.wantErr == nil {
+				require.NoError(t, err)
+			} else {
+				require.EqualError(t, err, tc.wantErr.Error())
+			}
+		})
+	}
+}

internal/pkg/template/template_functions.go

@@ -142,7 +142,11 @@ func generatePublishJSON(topics []*Topics) string {
 		if aws.StringValue(pb.Name) == "" {
 			continue
 		}
-		publisherMap[aws.StringValue(pb.Name)] = aws.StringValueSlice(pb.AllowedWorkers)
+		if pb.AllowedWorkers == nil {
+			publisherMap[aws.StringValue(pb.Name)] = []string{}
+		} else {
+			publisherMap[aws.StringValue(pb.Name)] = pb.AllowedWorkers
+		}
 	}
 
 	out, ok := getJSONMap(publisherMap)

internal/pkg/template/template_functions_test.go

@@ -244,7 +244,7 @@ func TestGeneratePublishJSON(t *testing.T) {
 			[]*Topics{
 				{
 					Name:           aws.String("tests"),
-					AllowedWorkers: aws.StringSlice([]string{"testsWorker1", "testsWorker2"}),
+					AllowedWorkers: []string{"testsWorker1", "testsWorker2"},
 				},
 			},
 		), "JSON should render correctly")

internal/pkg/template/workload.go

@@ -212,7 +212,7 @@ type PublishOpts struct {
 // Topics holds information needed to render a SNSTopic in a container definition.
 type Topics struct {
 	Name           *string
-	AllowedWorkers []*string
+	AllowedWorkers []string
 }
 
 // NetworkOpts holds AWS networking configuration for the workloads.