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
serviceversion states as part of the filtering mechanism. Nearly all
commands allow both 'active' and 'locked' serviceversions, and
previously 'service-version activate' blocked both, but it should
allow 'locked' while still blocking 'active'.

Additionally, the error message emitted when the specified
serviceversion 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' serviceversions in many
of the commands which specify filters.

.tmpl/create.go

@@ -62,6 +62,8 @@ type CreateCommand struct {
 // Exec invokes the application logic for the command.
 func (c *CreateCommand) Exec(in io.Reader, out io.Writer) error {
 	serviceID, serviceVersion, err := argparser.ServiceDetails(argparser.ServiceDetailsOpts{
+		Active:             optional.Of(false),
+		Locked:             optional.Of(false),
 		AutoCloneFlag:      c.autoClone,
 		Client:             c.Globals.Client,
 		Manifest:           c.manifest,

.tmpl/delete.go

@@ -62,6 +62,8 @@ type DeleteCommand struct {
 // Exec invokes the application logic for the command.
 func (c *DeleteCommand) Exec(in io.Reader, out io.Writer) error {
 	serviceID, serviceVersion, err := argparser.ServiceDetails(argparser.ServiceDetailsOpts{
+		Active:             optional.Of(false),
+		Locked:             optional.Of(false),
 		AutoCloneFlag:      c.autoClone,
 		Client:             c.Globals.Client,
 		Manifest:           c.manifest,

.tmpl/describe.go

@@ -57,7 +57,6 @@ 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,
 		Client:             c.Globals.Client,
 		Manifest:           c.manifest,
 		Out:                out,

.tmpl/list.go

@@ -58,7 +58,6 @@ 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,
 		Client:             c.Globals.Client,
 		Manifest:           c.manifest,
 		Out:                out,

.tmpl/test.go

@@ -25,12 +25,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:      "--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:      "--service-id 123 --version 2",
+			WantError: "service version 2 is locked",
 		},
 		{
 			Name: "validate Create${CLI_API} API error",
@@ -88,12 +96,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:      "--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:      "--service-id 123 --version 2",
+			WantError: "service version 2 is locked",
 		},
 		{
 			Name: "validate Delete${CLI_API} API error",
@@ -233,12 +249,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:      "--service-id 123 --version 1",
-			WantError: "service version 1 is not editable",
+			Args:      "--name foobar --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:      "--name foobar --service-id 123 --version 2",
+			WantError: "service version 2 is locked",
 		},
 		{
 			Name: "validate Update${CLI_API} API error",

.tmpl/update.go

@@ -66,6 +66,8 @@ type UpdateCommand struct {
 // Exec invokes the application logic for the command.
 func (c *UpdateCommand) Exec(in io.Reader, out io.Writer) error {
 	serviceID, serviceVersion, err := argparser.ServiceDetails(argparser.ServiceDetailsOpts{
+		Active:             optional.Of(false),
+		Locked:             optional.Of(false),
 		AutoCloneFlag:      c.autoClone,
 		Client:             c.Globals.Client,
 		Manifest:           c.manifest,

pkg/argparser/cmd.go

@@ -13,6 +13,7 @@ import (
 	"github.com/fastly/cli/pkg/global"
 	"github.com/fastly/cli/pkg/manifest"
 	"github.com/fastly/cli/pkg/text"
+	optional "github.com/leighmcculloch/go-optional"
 )
 
 // Command is an interface that abstracts over all of the concrete command
@@ -108,7 +109,14 @@ type OptionalFloat64 struct {
 // ServiceDetailsOpts provides data and behaviours required by the
 // ServiceDetails function.
 type ServiceDetailsOpts struct {
-	AllowActiveLocked  bool
+	// Active controls whether active serviceversions will be included in the result;
+	// if this is Empty, then the 'active' state of the version is ignored;
+	// otherwise, the 'active' state must match the value
+	Active optional.Optional[bool]
+	// Locked controls whether locked serviceversions will be included in the result;
+	// if this is Empty, then the 'locked' state of the version is ignored;
+	// otherwise, the 'locked' state must match the value
+	Locked             optional.Optional[bool]
 	AutoCloneFlag      OptionalAutoClone
 	APIClient          api.Interface
 	Manifest           manifest.Data
@@ -140,14 +148,41 @@ 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)) {
+		return serviceID, v, nil
+	}
+
+	failure := false
+	var failureState string
+
+	if active, present := opts.Active.Get(); present {
+		if active && !fastly.ToValue(v.Active) {
+			failure = true
+			failureState = "not active"
+		}
+		if !active && fastly.ToValue(v.Active) {
+			failure = true
+			failureState = "active"
+		}
+	}
+
+	if locked, present := opts.Locked.Get(); present {
+		if locked && !fastly.ToValue(v.Locked) {
+			failure = true
+			failureState = "not locked"
+		}
+		if !locked && fastly.ToValue(v.Locked) {
+			failure = true
+			failureState = "locked"
+		}
+	}
+
+	if failure {
 		err = fsterr.RemediationError{
-			Inner:       fmt.Errorf("service version %d is not editable", fastly.ToValue(v.Number)),
+			Inner:       fmt.Errorf("service version %d is %s", fastly.ToValue(v.Number), failureState),
 			Remediation: fsterr.AutoCloneRemediation,
 		}
 		return serviceID, v, err
 	}
-
 	return serviceID, v, nil
 }
 

pkg/commands/acl/acl_test.go

@@ -28,12 +28,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,
 			},
 			Arg:       "--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,
+			},
+			Arg:       "--name foo --service-id 123 --version 2",
+			WantError: "service version 2 is locked",
 		},
 		{
 			Name: "validate CreateACL API error",
@@ -102,12 +110,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,
 			},
 			Arg:       "--name foobar --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,
+			},
+			Arg:       "--name foo --service-id 123 --version 2",
+			WantError: "service version 2 is locked",
 		},
 		{
 			Name: "validate DeleteACL API error",
@@ -276,12 +292,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,
+			},
+			Arg:       "--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,
 			},
-			Arg:       "--name foobar --new-name beepboop --service-id 123 --version 1",
-			WantError: "service version 1 is not editable",
+			Arg:       "--name foo --new-name beepboop --service-id 123 --version 2",
+			WantError: "service version 2 is locked",
 		},
 		{
 			Name: "validate UpdateACL API error",

pkg/commands/acl/create.go

@@ -9,6 +9,7 @@ import (
 	"github.com/fastly/cli/pkg/errors"
 	"github.com/fastly/cli/pkg/global"
 	"github.com/fastly/cli/pkg/text"
+	optional "github.com/leighmcculloch/go-optional"
 )
 
 // NewCreateCommand returns a usable command registered under the parent.
@@ -64,6 +65,8 @@ type CreateCommand struct {
 // Exec invokes the application logic for the command.
 func (c *CreateCommand) Exec(_ io.Reader, out io.Writer) error {
 	serviceID, serviceVersion, err := argparser.ServiceDetails(argparser.ServiceDetailsOpts{
+		Active:             optional.Of(false),
+		Locked:             optional.Of(false),
 		AutoCloneFlag:      c.autoClone,
 		APIClient:          c.Globals.APIClient,
 		ErrLog:             c.Globals.ErrLog,

pkg/commands/acl/delete.go

@@ -9,6 +9,7 @@ import (
 	"github.com/fastly/cli/pkg/errors"
 	"github.com/fastly/cli/pkg/global"
 	"github.com/fastly/cli/pkg/text"
+	optional "github.com/leighmcculloch/go-optional"
 )
 
 // NewDeleteCommand returns a usable command registered under the parent.
@@ -63,6 +64,8 @@ type DeleteCommand struct {
 // Exec invokes the application logic for the command.
 func (c *DeleteCommand) Exec(_ io.Reader, out io.Writer) error {
 	serviceID, serviceVersion, err := argparser.ServiceDetails(argparser.ServiceDetailsOpts{
+		Active:             optional.Of(false),
+		Locked:             optional.Of(false),
 		AutoCloneFlag:      c.autoClone,
 		APIClient:          c.Globals.APIClient,
 		Manifest:           *c.Globals.Manifest,

pkg/commands/acl/describe.go

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

pkg/commands/acl/list.go

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

pkg/commands/acl/update.go

@@ -9,6 +9,7 @@ import (
 	"github.com/fastly/cli/pkg/errors"
 	"github.com/fastly/cli/pkg/global"
 	"github.com/fastly/cli/pkg/text"
+	optional "github.com/leighmcculloch/go-optional"
 )
 
 // NewUpdateCommand returns a usable command registered under the parent.
@@ -65,6 +66,8 @@ type UpdateCommand struct {
 // Exec invokes the application logic for the command.
 func (c *UpdateCommand) Exec(_ io.Reader, out io.Writer) error {
 	serviceID, serviceVersion, err := argparser.ServiceDetails(argparser.ServiceDetailsOpts{
+		Active:             optional.Of(false),
+		Locked:             optional.Of(false),
 		AutoCloneFlag:      c.autoClone,
 		APIClient:          c.Globals.APIClient,
 		Manifest:           *c.Globals.Manifest,

pkg/commands/backend/backend_test.go

@@ -30,9 +30,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.
+		{
+			Arg: "--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.
+		{
+			Arg: "--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.
 		{
 			Arg: "--service-id 123 --version 1 --address example.com --name www.test.com --autoclone",
@@ -122,8 +144,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.
 		{
 			Arg: "--service-id 123 --version 3 --address 127.0.0.1 --name www.test.com",
 			API: mock.API{

pkg/commands/backend/create.go

@@ -11,6 +11,7 @@ import (
 	fsterr "github.com/fastly/cli/pkg/errors"
 	"github.com/fastly/cli/pkg/global"
 	"github.com/fastly/cli/pkg/text"
+	optional "github.com/leighmcculloch/go-optional"
 )
 
 // CreateCommand calls the Fastly API to create backends.
@@ -127,6 +128,8 @@ func NewCreateCommand(parent argparser.Registerer, g *global.Data) *CreateComman
 // Exec invokes the application logic for the command.
 func (c *CreateCommand) Exec(_ io.Reader, out io.Writer) error {
 	serviceID, serviceVersion, err := argparser.ServiceDetails(argparser.ServiceDetailsOpts{
+		Active:             optional.Of(false),
+		Locked:             optional.Of(false),
 		AutoCloneFlag:      c.autoClone,
 		APIClient:          c.Globals.APIClient,
 		Manifest:           *c.Globals.Manifest,

pkg/commands/backend/delete.go

@@ -9,6 +9,7 @@ import (
 	"github.com/fastly/cli/pkg/errors"
 	"github.com/fastly/cli/pkg/global"
 	"github.com/fastly/cli/pkg/text"
+	optional "github.com/leighmcculloch/go-optional"
 )
 
 // DeleteCommand calls the Fastly API to delete backends.
@@ -62,6 +63,8 @@ func NewDeleteCommand(parent argparser.Registerer, g *global.Data) *DeleteComman
 // Exec invokes the application logic for the command.
 func (c *DeleteCommand) Exec(_ io.Reader, out io.Writer) error {
 	serviceID, serviceVersion, err := argparser.ServiceDetails(argparser.ServiceDetailsOpts{
+		Active:             optional.Of(false),
+		Locked:             optional.Of(false),
 		AutoCloneFlag:      c.autoClone,
 		APIClient:          c.Globals.APIClient,
 		Manifest:           *c.Globals.Manifest,