fix(service-version): Allow 'locked' services to be activated.

This PR changes the ServiceDetailsOpts structure (used by the
ServiceDetails function) to permit commands to specify individual
service states as part of the filtering mechanism. Nearly all commands
allow both 'active' and 'locked' services, and previously
'service-version activate' blocked both, but it should allow 'locked'
while still blocking 'active'.

Additionally, the error message emitted when the specified service is
not allowed by the filter now indicates whether it was the 'active' or
'locked' status which caused the error.

Finally, many more tests were added to ensure that the proper error
messages are emitted for 'active' and 'locked' services in all of the
commands which specify filters.

.tmpl/describe.go

@@ -57,7 +57,7 @@ type DescribeCommand struct {
 // Exec invokes the application logic for the command.
 func (c *DescribeCommand) Exec(in io.Reader, out io.Writer) error {
 	serviceID, serviceVersion, err := argparser.ServiceDetails(argparser.ServiceDetailsOpts{
-		AllowActiveLocked:  true,
+		DisallowStates:     argparser.DisallowStateActive | argparser.DisallowStateLocked,
 		Client:             c.Globals.Client,
 		Manifest:           c.manifest,
 		Out:                out,

.tmpl/list.go

@@ -58,7 +58,7 @@ type ListCommand struct {
 // Exec invokes the application logic for the command.
 func (c *ListCommand) Exec(in io.Reader, out io.Writer) error {
 	serviceID, serviceVersion, err := argparser.ServiceDetails(argparser.ServiceDetailsOpts{
-		AllowActiveLocked:  true,
+		DisallowStates:     argparser.DisallowStateActive | argparser.DisallowStateLocked,
 		Client:             c.Globals.Client,
 		Manifest:           c.manifest,
 		Out:                out,

.tmpl/test.go

@@ -24,12 +24,20 @@ func TestCreate(t *testing.T) {
 			WantError: "error reading service: no service ID found",
 		},
 		{
-			Name: "validate missing --autoclone flag",
+			Name: "validate missing --autoclone flag with 'active' service",
 			API: mock.API{
 				ListVersionsFn: testutil.ListVersions,
 			},
-			Args:      args("${CLI_COMMAND} create --service-id 123 --version 1"),
-			WantError: "service version 1 is not editable",
+			Args:      args("${CLI_COMMAND} --name foo --service-id 123 --version 1"),
+			WantError: "service version 1 is active",
+		},
+		{
+			Name: "validate missing --autoclone flag with 'locked' service",
+			API: mock.API{
+				ListVersionsFn: testutil.ListVersions,
+			},
+			Args:      args("${CLI_COMMAND} --name foo --service-id 123 --version 2"),
+			WantError: "service version 2 is locked",
 		},
 		{
 			Name: "validate Create${CLI_API} API error",
@@ -98,12 +106,20 @@ func TestDelete(t *testing.T) {
 			WantError: "error reading service: no service ID found",
 		},
 		{
-			Name: "validate missing --autoclone flag",
+			Name: "validate missing --autoclone flag with 'active' service",
 			API: mock.API{
 				ListVersionsFn: testutil.ListVersions,
 			},
-			Args:      args("${CLI_API} delete ---service-id 123 --version 1"),
-			WantError: "service version 1 is not editable",
+			Args:      args("${CLI_COMMAND} --name foo --service-id 123 --version 1"),
+			WantError: "service version 1 is active",
+		},
+		{
+			Name: "validate missing --autoclone flag with 'locked' service",
+			API: mock.API{
+				ListVersionsFn: testutil.ListVersions,
+			},
+			Args:      args("${CLI_COMMAND} --name foo --service-id 123 --version 2"),
+			WantError: "service version 2 is locked",
 		},
 		{
 			Name: "validate Delete${CLI_API} API error",
@@ -293,12 +309,20 @@ func TestUpdate(t *testing.T) {
 			WantError: "error reading service: no service ID found",
 		},
 		{
-			Name: "validate missing --autoclone flag",
+			Name: "validate missing --autoclone flag with 'active' service",
+			API: mock.API{
+				ListVersionsFn: testutil.ListVersions,
+			},
+			Args:      args("${CLI_COMMAND} --name foo --service-id 123 --version 1"),
+			WantError: "service version 1 is active",
+		},
+		{
+			Name: "validate missing --autoclone flag with 'locked' service",
 			API: mock.API{
 				ListVersionsFn: testutil.ListVersions,
 			},
-			Args:      args("${CLI_COMMAND} update --name foobar --service-id 123 --version 1"),
-			WantError: "service version 1 is not editable",
+			Args:      args("${CLI_COMMAND} --name foo --service-id 123 --version 2"),
+			WantError: "service version 2 is locked",
 		},
 		{
 			Name: "validate Update${CLI_API} API error",

pkg/argparser/cmd.go

@@ -105,10 +105,27 @@ type OptionalFloat64 struct {
 	Value float64
 }
 
+// DisallowState enumerates the service states which are permitted in a
+// lookup in ServiceDetails
+type DisallowState uint8
+
+const (
+	// DisallowStateNone means that no states are disallowed
+	DisallowStateNone DisallowState = 0
+	// DisallowStateActive means that 'active' services are disallowed
+	DisallowStateActive = 1 << iota
+	// DisallowStateLocked means that 'locked' services are disallowed
+	DisallowStateLocked
+)
+
+func (in DisallowState) has(state DisallowState) bool {
+	return in&state == state
+}
+
 // ServiceDetailsOpts provides data and behaviours required by the
 // ServiceDetails function.
 type ServiceDetailsOpts struct {
-	AllowActiveLocked  bool
+	DisallowStates     DisallowState
 	AutoCloneFlag      OptionalAutoClone
 	APIClient          api.Interface
 	Manifest           manifest.Data
@@ -140,9 +157,15 @@ func ServiceDetails(opts ServiceDetailsOpts) (serviceID string, serviceVersion *
 		if err != nil {
 			return serviceID, currentVersion, err
 		}
-	} else if !opts.AllowActiveLocked && (fastly.ToValue(v.Active) || fastly.ToValue(v.Locked)) {
+	} else if !opts.DisallowStates.has(DisallowStateActive) && fastly.ToValue(v.Active) {
+		err = fsterr.RemediationError{
+			Inner:       fmt.Errorf("service version %d is active", fastly.ToValue(v.Number)),
+			Remediation: fsterr.AutoCloneRemediation,
+		}
+		return serviceID, v, err
+	} else if !opts.DisallowStates.has(DisallowStateLocked) && fastly.ToValue(v.Locked) {
 		err = fsterr.RemediationError{
-			Inner:       fmt.Errorf("service version %d is not editable", fastly.ToValue(v.Number)),
+			Inner:       fmt.Errorf("service version %d is locked", fastly.ToValue(v.Number)),
 			Remediation: fsterr.AutoCloneRemediation,
 		}
 		return serviceID, v, err

pkg/commands/acl/acl_test.go

@@ -32,12 +32,20 @@ func TestACLCreate(t *testing.T) {
 			WantError: "error reading service: no service ID found",
 		},
 		{
-			Name: "validate missing --autoclone flag",
+			Name: "validate missing --autoclone flag with 'active' service",
 			API: mock.API{
 				ListVersionsFn: testutil.ListVersions,
 			},
 			Args:      args("acl create --name foo --service-id 123 --version 1"),
-			WantError: "service version 1 is not editable",
+			WantError: "service version 1 is active",
+		},
+		{
+			Name: "validate missing --autoclone flag with 'locked' service",
+			API: mock.API{
+				ListVersionsFn: testutil.ListVersions,
+			},
+			Args:      args("acl create --name foo --service-id 123 --version 2"),
+			WantError: "service version 2 is locked",
 		},
 		{
 			Name: "validate CreateACL API error",
@@ -121,12 +129,20 @@ func TestACLDelete(t *testing.T) {
 			WantError: "error reading service: no service ID found",
 		},
 		{
-			Name: "validate missing --autoclone flag",
+			Name: "validate missing --autoclone flag with 'active' service",
 			API: mock.API{
 				ListVersionsFn: testutil.ListVersions,
 			},
-			Args:      args("acl delete --name foobar --service-id 123 --version 1"),
-			WantError: "service version 1 is not editable",
+			Args:      args("acl delete --name foo --service-id 123 --version 1"),
+			WantError: "service version 1 is active",
+		},
+		{
+			Name: "validate missing --autoclone flag with 'locked' service",
+			API: mock.API{
+				ListVersionsFn: testutil.ListVersions,
+			},
+			Args:      args("acl delete --name foo --service-id 123 --version 2"),
+			WantError: "service version 2 is locked",
 		},
 		{
 			Name: "validate DeleteACL API error",
@@ -338,12 +354,20 @@ func TestACLUpdate(t *testing.T) {
 			WantError: "error reading service: no service ID found",
 		},
 		{
-			Name: "validate missing --autoclone flag",
+			Name: "validate missing --autoclone flag with 'active' service",
+			API: mock.API{
+				ListVersionsFn: testutil.ListVersions,
+			},
+			Args:      args("acl update --name foo --new-name beepboop --service-id 123 --version 1"),
+			WantError: "service version 1 is active",
+		},
+		{
+			Name: "validate missing --autoclone flag with 'locked' service",
 			API: mock.API{
 				ListVersionsFn: testutil.ListVersions,
 			},
-			Args:      args("acl update --name foobar --new-name beepboop --service-id 123 --version 1"),
-			WantError: "service version 1 is not editable",
+			Args:      args("acl update --name foo --new-name beepboop --service-id 123 --version 2"),
+			WantError: "service version 2 is locked",
 		},
 		{
 			Name: "validate UpdateACL API error",

pkg/commands/acl/describe.go

@@ -64,7 +64,7 @@ func (c *DescribeCommand) Exec(_ io.Reader, out io.Writer) error {
 	}
 
 	serviceID, serviceVersion, err := argparser.ServiceDetails(argparser.ServiceDetailsOpts{
-		AllowActiveLocked:  true,
+		DisallowStates:     argparser.DisallowStateActive | argparser.DisallowStateLocked,
 		APIClient:          c.Globals.APIClient,
 		Manifest:           *c.Globals.Manifest,
 		Out:                out,

pkg/commands/acl/list.go

@@ -63,7 +63,7 @@ func (c *ListCommand) Exec(_ io.Reader, out io.Writer) error {
 	}
 
 	serviceID, serviceVersion, err := argparser.ServiceDetails(argparser.ServiceDetailsOpts{
-		AllowActiveLocked:  true,
+		DisallowStates:     argparser.DisallowStateActive | argparser.DisallowStateLocked,
 		APIClient:          c.Globals.APIClient,
 		Manifest:           *c.Globals.Manifest,
 		Out:                out,

pkg/commands/backend/backend_test.go

@@ -33,9 +33,31 @@ func TestBackendCreate(t *testing.T) {
 			API: mock.API{
 				ListVersionsFn: testutil.ListVersions,
 			},
-			WantError: "service version 1 is not editable",
+			WantError: "service version 1 is active",
 		},
-		// The following test is the same as the above but it appends --autoclone
+		// The following test specifies a service version that's 'locked', and
+		// subsequently we expect it to not be cloned as we don't provide the
+		// --autoclone flag and trying to add a backend to an activated service
+		// should cause an error.
+		{
+			Args: args("backend create --service-id 123 --version 2 --address example.com --name www.test.com"),
+			API: mock.API{
+				ListVersionsFn: testutil.ListVersions,
+			},
+			WantError: "service version 2 is locked",
+		},
+		// The following test is the same as the 'active' test above but it appends --autoclone
+		// so we can be sure the backend creation error still occurs.
+		{
+			Args: args("backend create --service-id 123 --version 1 --address example.com --name www.test.com --autoclone"),
+			API: mock.API{
+				ListVersionsFn:  testutil.ListVersions,
+				CloneVersionFn:  testutil.CloneVersionResult(4),
+				CreateBackendFn: createBackendError,
+			},
+			WantError: errTest.Error(),
+		},
+		// The following test is the same as the 'locked' test above but it appends --autoclone
 		// so we can be sure the backend creation error still occurs.
 		{
 			Args: args("backend create --service-id 123 --version 1 --address example.com --name www.test.com --autoclone"),
@@ -125,8 +147,8 @@ func TestBackendCreate(t *testing.T) {
 			},
 			WantOutput: "Created backend www.test.com (service 123 version 4)",
 		},
-		// The following test specifies a service version that's 'inactive', and
-		// subsequently we expect it to be the same editable version.
+		// The following test specifies a service version that's 'inactive' and not 'locked',
+		// and subsequently we expect it to be the same editable version.
 		{
 			Args: args("backend create --service-id 123 --version 3 --address 127.0.0.1 --name www.test.com"),
 			API: mock.API{

pkg/commands/backend/describe.go

@@ -63,7 +63,7 @@ func (c *DescribeCommand) Exec(_ io.Reader, out io.Writer) error {
 	}
 
 	serviceID, serviceVersion, err := argparser.ServiceDetails(argparser.ServiceDetailsOpts{
-		AllowActiveLocked:  true,
+		DisallowStates:     argparser.DisallowStateActive | argparser.DisallowStateLocked,
 		APIClient:          c.Globals.APIClient,
 		Manifest:           *c.Globals.Manifest,
 		Out:                out,

pkg/commands/backend/list.go

@@ -63,7 +63,7 @@ func (c *ListCommand) Exec(_ io.Reader, out io.Writer) error {
 	}
 
 	serviceID, serviceVersion, err := argparser.ServiceDetails(argparser.ServiceDetailsOpts{
-		AllowActiveLocked:  true,
+		DisallowStates:     argparser.DisallowStateActive | argparser.DisallowStateLocked,
 		APIClient:          c.Globals.APIClient,
 		Manifest:           *c.Globals.Manifest,
 		Out:                out,

pkg/commands/compute/deploy_test.go

@@ -198,6 +198,17 @@ func TestDeploy(t *testing.T) {
 			},
 			wantError: fmt.Sprintf("error cloning service version: %s", testutil.Err.Error()),
 		},
+		{
+			name: "service version is locked, clone version error",
+			args: args("compute deploy --service-id 123 --token 123 --version 2"),
+			api: mock.API{
+				CloneVersionFn:      testutil.CloneVersionError,
+				GetPackageFn:        getPackageOk,
+				GetServiceDetailsFn: getServiceDetailsWasm,
+				ListVersionsFn:      testutil.ListVersions,
+			},
+			wantError: fmt.Sprintf("error cloning service version: %s", testutil.Err.Error()),
+		},
 		{
 			name: "list domains error",
 			args: args("compute deploy --service-id 123 --token 123"),

pkg/commands/dictionary/describe.go

@@ -63,7 +63,7 @@ func (c *DescribeCommand) Exec(_ io.Reader, out io.Writer) error {
 	}
 
 	serviceID, serviceVersion, err := argparser.ServiceDetails(argparser.ServiceDetailsOpts{
-		AllowActiveLocked:  true,
+		DisallowStates:     argparser.DisallowStateActive | argparser.DisallowStateLocked,
 		APIClient:          c.Globals.APIClient,
 		Manifest:           *c.Globals.Manifest,
 		Out:                out,

pkg/commands/dictionary/list.go

@@ -62,7 +62,7 @@ func (c *ListCommand) Exec(_ io.Reader, out io.Writer) error {
 		return fsterr.ErrInvalidVerboseJSONCombo
 	}
 	serviceID, serviceVersion, err := argparser.ServiceDetails(argparser.ServiceDetailsOpts{
-		AllowActiveLocked:  true,
+		DisallowStates:     argparser.DisallowStateActive | argparser.DisallowStateLocked,
 		APIClient:          c.Globals.APIClient,
 		Manifest:           *c.Globals.Manifest,
 		Out:                out,

pkg/commands/domain/describe.go

@@ -63,7 +63,7 @@ func (c *DescribeCommand) Exec(_ io.Reader, out io.Writer) error {
 	}
 
 	serviceID, serviceVersion, err := argparser.ServiceDetails(argparser.ServiceDetailsOpts{
-		AllowActiveLocked:  true,
+		DisallowStates:     argparser.DisallowStateActive | argparser.DisallowStateLocked,
 		APIClient:          c.Globals.APIClient,
 		Manifest:           *c.Globals.Manifest,
 		Out:                out,

pkg/commands/domain/list.go

@@ -63,7 +63,7 @@ func (c *ListCommand) Exec(_ io.Reader, out io.Writer) error {
 	}
 
 	serviceID, serviceVersion, err := argparser.ServiceDetails(argparser.ServiceDetailsOpts{
-		AllowActiveLocked:  true,
+		DisallowStates:     argparser.DisallowStateActive | argparser.DisallowStateLocked,
 		APIClient:          c.Globals.APIClient,
 		Manifest:           *c.Globals.Manifest,
 		Out:                out,

pkg/commands/domain/validate.go

@@ -57,7 +57,7 @@ type ValidateCommand struct {
 // Exec invokes the application logic for the command.
 func (c *ValidateCommand) Exec(_ io.Reader, out io.Writer) error {
 	serviceID, serviceVersion, err := argparser.ServiceDetails(argparser.ServiceDetailsOpts{
-		AllowActiveLocked:  true,
+		DisallowStates:     argparser.DisallowStateActive | argparser.DisallowStateLocked,
 		APIClient:          c.Globals.APIClient,
 		Manifest:           *c.Globals.Manifest,
 		Out:                out,

pkg/commands/healthcheck/describe.go

@@ -64,7 +64,7 @@ func (c *DescribeCommand) Exec(_ io.Reader, out io.Writer) error {
 	}
 
 	serviceID, serviceVersion, err := argparser.ServiceDetails(argparser.ServiceDetailsOpts{
-		AllowActiveLocked:  true,
+		DisallowStates:     argparser.DisallowStateActive | argparser.DisallowStateLocked,
 		APIClient:          c.Globals.APIClient,
 		Manifest:           *c.Globals.Manifest,
 		Out:                out,

pkg/commands/healthcheck/list.go

@@ -63,7 +63,7 @@ func (c *ListCommand) Exec(_ io.Reader, out io.Writer) error {
 	}
 
 	serviceID, serviceVersion, err := argparser.ServiceDetails(argparser.ServiceDetailsOpts{
-		AllowActiveLocked:  true,
+		DisallowStates:     argparser.DisallowStateActive | argparser.DisallowStateLocked,
 		APIClient:          c.Globals.APIClient,
 		Manifest:           *c.Globals.Manifest,
 		Out:                out,

pkg/commands/logging/azureblob/describe.go

@@ -63,7 +63,7 @@ func (c *DescribeCommand) Exec(_ io.Reader, out io.Writer) error {
 	}
 
 	serviceID, serviceVersion, err := argparser.ServiceDetails(argparser.ServiceDetailsOpts{
-		AllowActiveLocked:  true,
+		DisallowStates:     argparser.DisallowStateActive | argparser.DisallowStateLocked,
 		APIClient:          c.Globals.APIClient,
 		Manifest:           *c.Globals.Manifest,
 		Out:                out,

pkg/commands/logging/azureblob/list.go

@@ -63,7 +63,7 @@ func (c *ListCommand) Exec(_ io.Reader, out io.Writer) error {
 	}
 
 	serviceID, serviceVersion, err := argparser.ServiceDetails(argparser.ServiceDetailsOpts{
-		AllowActiveLocked:  true,
+		DisallowStates:     argparser.DisallowStateActive | argparser.DisallowStateLocked,
 		APIClient:          c.Globals.APIClient,
 		Manifest:           *c.Globals.Manifest,
 		Out:                out,

pkg/commands/logging/bigquery/describe.go

@@ -63,7 +63,7 @@ func (c *DescribeCommand) Exec(_ io.Reader, out io.Writer) error {
 	}
 
 	serviceID, serviceVersion, err := argparser.ServiceDetails(argparser.ServiceDetailsOpts{
-		AllowActiveLocked:  true,
+		DisallowStates:     argparser.DisallowStateActive | argparser.DisallowStateLocked,
 		APIClient:          c.Globals.APIClient,
 		Manifest:           *c.Globals.Manifest,
 		Out:                out,