Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(all commands): --service-name flag should have priority. #1264

Merged
merged 3 commits into from
Aug 1, 2024
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .tmpl/create.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ func NewCreateCommand(parent argparser.Registerer, globals *config.Data, data ma
c.RegisterFlag(argparser.StringFlagOpts{
Action: c.serviceName.Set,
Name: argparser.FlagServiceName,
Description: argparser.FlagServiceDesc,
Description: argparser.FlagServiceNameDesc,
Dst: &c.serviceName.Value,
})

Expand Down
2 changes: 1 addition & 1 deletion .tmpl/delete.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ func NewDeleteCommand(parent argparser.Registerer, globals *config.Data, data ma
c.RegisterFlag(argparser.StringFlagOpts{
Action: c.serviceName.Set,
Name: argparser.FlagServiceName,
Description: argparser.FlagServiceDesc,
Description: argparser.FlagServiceNameDesc,
Dst: &c.serviceName.Value,
})

Expand Down
2 changes: 1 addition & 1 deletion .tmpl/describe.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ func NewDescribeCommand(parent argparser.Registerer, globals *config.Data, data
c.RegisterFlag(argparser.StringFlagOpts{
Action: c.serviceName.Set,
Name: argparser.FlagServiceName,
Description: argparser.FlagServiceDesc,
Description: argparser.FlagServiceNameDesc,
Dst: &c.serviceName.Value,
})

Expand Down
2 changes: 1 addition & 1 deletion .tmpl/list.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ func NewListCommand(parent argparser.Registerer, globals *config.Data, data mani
c.RegisterFlag(argparser.StringFlagOpts{
Action: c.serviceName.Set,
Name: argparser.FlagServiceName,
Description: argparser.FlagServiceDesc,
Description: argparser.FlagServiceNameDesc,
Dst: &c.serviceName.Value,
})

Expand Down
2 changes: 1 addition & 1 deletion .tmpl/update.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ func NewUpdateCommand(parent argparser.Registerer, globals *config.Data, data ma
c.RegisterFlag(argparser.StringFlagOpts{
Action: c.serviceName.Set,
Name: argparser.FlagServiceName,
Description: argparser.FlagServiceDesc,
Description: argparser.FlagServiceNameDesc,
Dst: &c.serviceName.Value,
})

Expand Down
18 changes: 11 additions & 7 deletions pkg/argparser/cmd.go
Original file line number Diff line number Diff line change
Expand Up @@ -153,15 +153,15 @@ func ServiceDetails(opts ServiceDetailsOpts) (serviceID string, serviceVersion *

// ServiceID returns the Service ID and the source of that information.
//
// NOTE: If Service ID not provided then check if Service Name provided and use
// that information to acquire the Service ID.
// NOTE: If Service Name is provided it overrides all other methods of
// obtaining the Service ID.
func ServiceID(serviceName OptionalServiceNameID, data manifest.Data, client api.Interface, li fsterr.LogInterface) (serviceID string, source manifest.Source, flag string, err error) {
flag = "--service-id"
serviceID, source = data.ServiceID()

if source == manifest.SourceUndefined {
if !serviceName.WasSet {
err = fsterr.ErrNoServiceID
if serviceName.WasSet {
if source == manifest.SourceFlag {
err = fmt.Errorf("cannot specify both %s and %s", FlagServiceIDName, FlagServiceName)
if li != nil {
li.Add(err)
}
Expand All @@ -174,9 +174,13 @@ func ServiceID(serviceName OptionalServiceNameID, data manifest.Data, client api
if li != nil {
li.Add(err)
}
} else {
source = manifest.SourceFlag
return serviceID, source, flag, err
}
source = manifest.SourceFlag
}

if source == manifest.SourceUndefined {
err = fsterr.ErrNoServiceID
}

return serviceID, source, flag, err
Expand Down
4 changes: 2 additions & 2 deletions pkg/argparser/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@ var (
FlagServiceIDDesc = "Service ID (falls back to FASTLY_SERVICE_ID, then fastly.toml)"
// FlagServiceName is the flag name.
FlagServiceName = "service-name"
// FlagServiceDesc is the flag description.
FlagServiceDesc = "The name of the service"
// FlagServiceNameDesc is the flag description.
FlagServiceNameDesc = "The name of the service"
// FlagVersionName is the flag name.
FlagVersionName = "version"
// FlagVersionDesc is the flag description.
Expand Down
92 changes: 92 additions & 0 deletions pkg/argparser/flags_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ package argparser_test
import (
"bytes"
"fmt"
"io"
"net/http"
"sort"
"strconv"
"strings"
Expand All @@ -11,6 +13,7 @@ import (
"github.com/fastly/go-fastly/v9/fastly"

"github.com/fastly/cli/pkg/argparser"
"github.com/fastly/cli/pkg/manifest"
"github.com/fastly/cli/pkg/mock"
"github.com/fastly/cli/pkg/testutil"
)
Expand Down Expand Up @@ -318,6 +321,95 @@ func TestOptionalAutoCloneParse(t *testing.T) {
}
}

func TestServiceID(t *testing.T) {
cases := map[string]struct {
ServiceName argparser.OptionalServiceNameID
Data manifest.Data
API mock.API
WantServiceID string
WantError string
WantSource manifest.Source
WantFlag string
}{
"service-id flag": {
Data: manifest.Data{
Flag: manifest.Flag{ServiceID: "456"},
},
WantServiceID: "456",
WantSource: manifest.SourceFlag,
WantFlag: argparser.FlagServiceIDName,
},
"service ID in manifest": {
Data: manifest.Data{
File: manifest.File{ServiceID: "456"},
},
WantServiceID: "456",
WantSource: manifest.SourceFile,
},
"service-name flag with service-id flag": {
ServiceName: argparser.OptionalServiceNameID{argparser.OptionalString{Optional: argparser.Optional{WasSet: true}, Value: "bar"}},
Data: manifest.Data{
Flag: manifest.Flag{ServiceID: "123"},
},
WantError: "cannot specify both service-id and service-name",
},
"service-name flag with service-id in file": {
ServiceName: argparser.OptionalServiceNameID{argparser.OptionalString{Optional: argparser.Optional{WasSet: true}, Value: "bar"}},
Data: manifest.Data{
File: manifest.File{ServiceID: "123"},
},
API: mock.API{
GetServicesFn: func(i *fastly.GetServicesInput) *fastly.ListPaginator[fastly.Service] {
return fastly.NewPaginator[fastly.Service](&mock.HTTPClient{
Errors: []error{nil},
Responses: []*http.Response{
{
Body: io.NopCloser(strings.NewReader(`[{"id": "456", "name": "bar"}]`)),
},
},
}, fastly.ListOpts{}, "/example")
},
},
WantServiceID: "456",
WantSource: manifest.SourceFlag,
WantFlag: argparser.FlagServiceName,
},
"unknown service-name flag value": {
ServiceName: argparser.OptionalServiceNameID{argparser.OptionalString{Optional: argparser.Optional{WasSet: true}, Value: "bar"}},
Data: manifest.Data{},
API: mock.API{
GetServicesFn: func(i *fastly.GetServicesInput) *fastly.ListPaginator[fastly.Service] {
return fastly.NewPaginator[fastly.Service](&mock.HTTPClient{
Errors: []error{nil},
Responses: []*http.Response{
{
Body: io.NopCloser(strings.NewReader(`[{"id": "456", "name": "beepboop"}]`)),
},
},
}, fastly.ListOpts{}, "/example")
},
},
WantError: "error matching service name with available services",
},
"no information provided": {
Data: manifest.Data{},
WantError: "error reading service: no service ID found",
},
}

for name, c := range cases {
t.Run(name, func(t *testing.T) {
serviceID, source, flag, err := argparser.ServiceID(c.ServiceName, c.Data, c.API, nil)
testutil.AssertErrorContains(t, err, c.WantError)
if err == nil {
testutil.AssertString(t, serviceID, c.WantServiceID)
testutil.AssertStringContains(t, flag, c.WantFlag)
testutil.AssertEqual(t, source, c.WantSource)
}
})
}
}

// cloneVersionResult returns a function which returns a specific cloned version.
func cloneVersionResult(version int) func(i *fastly.CloneVersionInput) (*fastly.Version, error) {
return func(i *fastly.CloneVersionInput) (*fastly.Version, error) {
Expand Down
2 changes: 1 addition & 1 deletion pkg/commands/acl/create.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ func NewCreateCommand(parent argparser.Registerer, g *global.Data) *CreateComman
c.RegisterFlag(argparser.StringFlagOpts{
Action: c.serviceName.Set,
Name: argparser.FlagServiceName,
Description: argparser.FlagServiceDesc,
Description: argparser.FlagServiceNameDesc,
Dst: &c.serviceName.Value,
})

Expand Down
2 changes: 1 addition & 1 deletion pkg/commands/acl/delete.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ func NewDeleteCommand(parent argparser.Registerer, g *global.Data) *DeleteComman
c.RegisterFlag(argparser.StringFlagOpts{
Action: c.serviceName.Set,
Name: argparser.FlagServiceName,
Description: argparser.FlagServiceDesc,
Description: argparser.FlagServiceNameDesc,
Dst: &c.serviceName.Value,
})

Expand Down
2 changes: 1 addition & 1 deletion pkg/commands/acl/describe.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ func NewDescribeCommand(parent argparser.Registerer, g *global.Data) *DescribeCo
c.RegisterFlag(argparser.StringFlagOpts{
Action: c.serviceName.Set,
Name: argparser.FlagServiceName,
Description: argparser.FlagServiceDesc,
Description: argparser.FlagServiceNameDesc,
Dst: &c.serviceName.Value,
})

Expand Down
2 changes: 1 addition & 1 deletion pkg/commands/acl/list.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ func NewListCommand(parent argparser.Registerer, g *global.Data) *ListCommand {
c.RegisterFlag(argparser.StringFlagOpts{
Action: c.serviceName.Set,
Name: argparser.FlagServiceName,
Description: argparser.FlagServiceDesc,
Description: argparser.FlagServiceNameDesc,
Dst: &c.serviceName.Value,
})

Expand Down
2 changes: 1 addition & 1 deletion pkg/commands/acl/update.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ func NewUpdateCommand(parent argparser.Registerer, g *global.Data) *UpdateComman
c.RegisterFlag(argparser.StringFlagOpts{
Action: c.serviceName.Set,
Name: argparser.FlagServiceName,
Description: argparser.FlagServiceDesc,
Description: argparser.FlagServiceNameDesc,
Dst: &c.serviceName.Value,
})

Expand Down
2 changes: 1 addition & 1 deletion pkg/commands/aclentry/create.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ func NewCreateCommand(parent argparser.Registerer, g *global.Data) *CreateComman
c.RegisterFlag(argparser.StringFlagOpts{
Action: c.serviceName.Set,
Name: argparser.FlagServiceName,
Description: argparser.FlagServiceDesc,
Description: argparser.FlagServiceNameDesc,
Dst: &c.serviceName.Value,
})
c.CmdClause.Flag("subnet", "Number of bits for the subnet mask applied to the IP address").Action(c.subnet.Set).IntVar(&c.subnet.Value)
Expand Down
2 changes: 1 addition & 1 deletion pkg/commands/aclentry/delete.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ func NewDeleteCommand(parent argparser.Registerer, g *global.Data) *DeleteComman
c.RegisterFlag(argparser.StringFlagOpts{
Action: c.serviceName.Set,
Name: argparser.FlagServiceName,
Description: argparser.FlagServiceDesc,
Description: argparser.FlagServiceNameDesc,
Dst: &c.serviceName.Value,
})

Expand Down
2 changes: 1 addition & 1 deletion pkg/commands/aclentry/describe.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ func NewDescribeCommand(parent argparser.Registerer, g *global.Data) *DescribeCo
c.RegisterFlag(argparser.StringFlagOpts{
Action: c.serviceName.Set,
Name: argparser.FlagServiceName,
Description: argparser.FlagServiceDesc,
Description: argparser.FlagServiceNameDesc,
Dst: &c.serviceName.Value,
})

Expand Down
2 changes: 1 addition & 1 deletion pkg/commands/aclentry/list.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ func NewListCommand(parent argparser.Registerer, g *global.Data) *ListCommand {
c.RegisterFlag(argparser.StringFlagOpts{
Action: c.serviceName.Set,
Name: argparser.FlagServiceName,
Description: argparser.FlagServiceDesc,
Description: argparser.FlagServiceNameDesc,
Dst: &c.serviceName.Value,
})

Expand Down
2 changes: 1 addition & 1 deletion pkg/commands/aclentry/update.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ func NewUpdateCommand(parent argparser.Registerer, g *global.Data) *UpdateComman
c.RegisterFlag(argparser.StringFlagOpts{
Action: c.serviceName.Set,
Name: argparser.FlagServiceName,
Description: argparser.FlagServiceDesc,
Description: argparser.FlagServiceNameDesc,
Dst: &c.serviceName.Value,
})
c.CmdClause.Flag("subnet", "Number of bits for the subnet mask applied to the IP address").Action(c.subnet.Set).IntVar(&c.subnet.Value)
Expand Down
2 changes: 1 addition & 1 deletion pkg/commands/backend/create.go
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ func NewCreateCommand(parent argparser.Registerer, g *global.Data) *CreateComman
c.RegisterFlag(argparser.StringFlagOpts{
Action: c.serviceName.Set,
Name: argparser.FlagServiceName,
Description: argparser.FlagServiceDesc,
Description: argparser.FlagServiceNameDesc,
Dst: &c.serviceName.Value,
})
c.CmdClause.Flag("shield", "The shield POP designated to reduce inbound load on this origin by serving the cached data to the rest of the network").Action(c.shield.Set).StringVar(&c.shield.Value)
Expand Down
2 changes: 1 addition & 1 deletion pkg/commands/backend/delete.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ func NewDeleteCommand(parent argparser.Registerer, g *global.Data) *DeleteComman
c.RegisterFlag(argparser.StringFlagOpts{
Action: c.serviceName.Set,
Name: argparser.FlagServiceName,
Description: argparser.FlagServiceDesc,
Description: argparser.FlagServiceNameDesc,
Dst: &c.serviceName.Value,
})

Expand Down
2 changes: 1 addition & 1 deletion pkg/commands/backend/describe.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ func NewDescribeCommand(parent argparser.Registerer, g *global.Data) *DescribeCo
c.RegisterFlag(argparser.StringFlagOpts{
Action: c.serviceName.Set,
Name: argparser.FlagServiceName,
Description: argparser.FlagServiceDesc,
Description: argparser.FlagServiceNameDesc,
Dst: &c.serviceName.Value,
})
return &c
Expand Down
2 changes: 1 addition & 1 deletion pkg/commands/backend/list.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ func NewListCommand(parent argparser.Registerer, g *global.Data) *ListCommand {
c.RegisterFlag(argparser.StringFlagOpts{
Action: c.serviceName.Set,
Name: argparser.FlagServiceName,
Description: argparser.FlagServiceDesc,
Description: argparser.FlagServiceNameDesc,
Dst: &c.serviceName.Value,
})
return &c
Expand Down
2 changes: 1 addition & 1 deletion pkg/commands/backend/update.go
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ func NewUpdateCommand(parent argparser.Registerer, g *global.Data) *UpdateComman
c.RegisterFlag(argparser.StringFlagOpts{
Action: c.serviceName.Set,
Name: argparser.FlagServiceName,
Description: argparser.FlagServiceDesc,
Description: argparser.FlagServiceNameDesc,
Dst: &c.serviceName.Value,
})
c.CmdClause.Flag("shield", "The shield POP designated to reduce inbound load on this origin by serving the cached data to the rest of the network").Action(c.Shield.Set).StringVar(&c.Shield.Value)
Expand Down
2 changes: 1 addition & 1 deletion pkg/commands/compute/deploy.go
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ func NewDeployCommand(parent argparser.Registerer, g *global.Data) *DeployComman
c.RegisterFlag(argparser.StringFlagOpts{
Action: c.ServiceName.Set,
Name: argparser.FlagServiceName,
Description: argparser.FlagServiceDesc,
Description: argparser.FlagServiceNameDesc,
Dst: &c.ServiceName.Value,
})
c.RegisterFlag(argparser.StringFlagOpts{
Expand Down
2 changes: 1 addition & 1 deletion pkg/commands/compute/publish.go
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ func NewPublishCommand(parent argparser.Registerer, g *global.Data, build *Build
c.RegisterFlag(argparser.StringFlagOpts{
Action: c.serviceName.Set,
Name: argparser.FlagServiceName,
Description: argparser.FlagServiceDesc,
Description: argparser.FlagServiceNameDesc,
Dst: &c.serviceName.Value,
})
c.CmdClause.Flag("status-check-code", "Set the expected status response for the service availability check to the root path").IntVar(&c.statusCheckCode)
Expand Down
2 changes: 1 addition & 1 deletion pkg/commands/compute/update.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ func NewUpdateCommand(parent argparser.Registerer, g *global.Data) *UpdateComman
c.RegisterFlag(argparser.StringFlagOpts{
Action: c.serviceName.Set,
Name: argparser.FlagServiceName,
Description: argparser.FlagServiceDesc,
Description: argparser.FlagServiceNameDesc,
Dst: &c.serviceName.Value,
})
c.RegisterFlag(argparser.StringFlagOpts{
Expand Down
2 changes: 1 addition & 1 deletion pkg/commands/dictionary/create.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ func NewCreateCommand(parent argparser.Registerer, g *global.Data) *CreateComman
c.RegisterFlag(argparser.StringFlagOpts{
Action: c.serviceName.Set,
Name: argparser.FlagServiceName,
Description: argparser.FlagServiceDesc,
Description: argparser.FlagServiceNameDesc,
Dst: &c.serviceName.Value,
})
c.CmdClause.Flag("write-only", "Whether to mark this dictionary as write-only").Action(c.writeOnly.Set).BoolVar(&c.writeOnly.Value)
Expand Down
2 changes: 1 addition & 1 deletion pkg/commands/dictionary/delete.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ func NewDeleteCommand(parent argparser.Registerer, g *global.Data) *DeleteComman
c.RegisterFlag(argparser.StringFlagOpts{
Action: c.serviceName.Set,
Name: argparser.FlagServiceName,
Description: argparser.FlagServiceDesc,
Description: argparser.FlagServiceNameDesc,
Dst: &c.serviceName.Value,
})
return &c
Expand Down
2 changes: 1 addition & 1 deletion pkg/commands/dictionary/describe.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ func NewDescribeCommand(parent argparser.Registerer, g *global.Data) *DescribeCo
c.RegisterFlag(argparser.StringFlagOpts{
Action: c.serviceName.Set,
Name: argparser.FlagServiceName,
Description: argparser.FlagServiceDesc,
Description: argparser.FlagServiceNameDesc,
Dst: &c.serviceName.Value,
})
return &c
Expand Down
Loading