8000 Support usage of IAM role in S3 and Kinesis logging endpoints by kellymclaughlin · Pull Request #253 · fastly/cli · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Support usage of IAM role in S3 and Kinesis logging endpoints #253

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

Merged
merged 2 commits into from
Apr 14, 2021
Merged
Show file tree
Hide file tree
Changes from all 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
3 changes: 1 addition & 2 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,12 @@ require (
github.com/alecthomas/units v0.0.0-20190924025748-f65c72e2690d // indirect
github.com/blang/semver v3.5.1+incompatible
github.com/dustinkirkland/golang-petname v0.0.0-20191129215211-8e5a1ed0cff0
github.com/fastly/go-fastly/v3 v3.0.0
github.com/fastly/go-fastly/v3 v3.5.0
github.com/fastly/kingpin v2.1.12-0.20191105091915-95d230a53780+incompatible
github.com/fatih/color v1.7.0
github.com/frankban/quicktest v1.5.0 // indirect
github.com/google/go-cmp v0.3.1
github.com/google/go-github/v28 v28.1.1
github.com/google/jsonapi v0.0.0-20200226002910-c8283f632fb7 // indirect
github.com/hashicorp/go-cleanhttp v0.5.1 // indirect
github.com/kennygrant/sanitize v1.2.4
github.com/mattn/go-colorable v0.1.7 // indirect
Expand Down
9 changes: 4 additions & 5 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,8 @@ github.com/dustinkirkland/golang-petname v0.0.0-20191129215211-8e5a1ed0cff0 h1:9
github.com/dustinkirkland/golang-petname v0.0.0-20191129215211-8e5a1ed0cff0/go.mod h1:V+Qd57rJe8gd4eiGzZyg4h54VLHmYVVw54iMnlAMrF8=
github.com/emirpasic/gods v1.12.0 h1:QAUIPSaCu4G+POclxeqb3F+WPpdKqFGlw36+yOzGlrg=
github.com/emirpasic/gods v1.12.0/go.mod h1:YfzfFFoVP/catgzJb4IKIqXjX78Ha8FMSDh3ymbK86o=
github.com/fastly/go-fastly/v3 v3.0.0 h1:TtTkBnBO5CcPTAUgBL/vcoS5VL5LaKfXV6cilX8bn8U=
github.com/fastly/go-fastly/v3 v3.0.0/go.mod h1:1GAnpEjDwBQD6DZgrgO5RoIawXFqrYC1K+JOkgZXCPk=
github.com/fastly/go-fastly/v3 v3.5.0 h1:NKLA21fapanz0s4Gun10XNWG34xkC5OwCgNc8IRMh5A=
github.com/fastly/go-fastly/v3 v3.5.0/go.mod h1:KOaCWsmkIKSASPzADl8PT/bTQIghOw/eEaxlHOu3jMA=
github.com/fastly/kingpin v2.1.12-0.20191105091915-95d230a53780+incompatible h1:FhrXlfhgGCS+uc6YwyiFUt04alnjpoX7vgDKJxS6Qbk=
github.com/fastly/kingpin v2.1.12-0.20191105091915-95d230a53780+incompatible/go.mod h1:U8UynVoU1SQaqD2I4ZqgYd5lx3A1ipQYn4aSt2Y5h6c=
github.com/fatih/color v1.7.0 h1:DkWD4oS2D8LGGgTQ6IvwJJXSL5Vp2ffcQg58nFV38Ys=
Expand All @@ -52,9 +52,8 @@ github.com/google/go-github/v28 v28.1.1 h1:kORf5ekX5qwXO2mGzXXOjMe/g6ap8ahVe0sBE
github.com/google/go-github/v28 v28.1.1/go.mod h1:bsqJWQX05omyWVmc00nEUql9mhQyv38lDZ8kPZcQVoM=
github.com/google/go-querystring v1.0.0 h1:Xkwi/a1rcvNg1PPYe5vI8GbeBY/jrVuDX5ASuANWTrk=
github.com/google/go-querystring v1.0.0/go.mod h1:odCYkC5MyYFN7vkCjXpyrEuKhc/BUO6wN/zVPAxq5ck=
github.com/google/jsonapi v0.0.0-20170708005851-46d3ced04344/go.mod h1:XSx4m2SziAqk9DXY9nz659easTq4q6TyrpYd9tHSm0g=
github.com/google/jsonapi v0.0.0-20200226002910-c8283f632fb7 h1:aQ4kMXDAmP9IRIZHcSKB2orXHGwGiSxH4PX1BzKHR50=
github.com/google/jsonapi v0.0.0-20200226002910-c8283f632fb7/go.mod h1:XSx4m2SziAqk9DXY9nz659easTq4q6TyrpYd9tHSm0g=
github.com/google/jsonapi v0.0.0-20201022225600-f822737867f6 h1:nVbdADVJLcaOp/CAR9xhaMCZrYn07HFFhUtM+dHsAIc=
github.com/google/jsonapi v0.0.0-20201022225600-f822737867f6/go.mod h1:XSx4m2SziAqk9DXY9nz659easTq4q6TyrpYd9tHSm0g=
github.com/google/renameio v0.1.0/go.mod h1:KWCgfxg9yswjAJkECMjeO8J8rahYeXnNhOm40UhjYkI=
github.com/hashicorp/go-cleanhttp v0.0.0-20170211013415-3573b8b52aa7/go.mod h1:JpRdi6/HCYpAwUzNwuwqhbovhLtngrth3wmdIIUrZ80=
github.com/hashicorp/go-cleanhttp v0.5.1 h1:dH3aiDG9Jvb5r5+bYHsikaOUIpcM0xvgMXVoDkXMzJM=
Expand Down
4 changes: 3 additions & 1 deletion pkg/app/run_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -760,7 +760,7 @@ COMMANDS
-n, --name=NAME The name of the BigQuery logging object
-s, --service-id=SERVICE-ID Service ID

logging s3 create --name=NAME --version=VERSION --bucket=BUCKET --access-key=ACCESS-KEY --secret-key=SECRET-KEY [<flags>]
logging s3 create --name=NAME --version=VERSION --bucket=BUCKET [<flags>]
Create an Amazon S3 logging endpoint on a Fastly service version

-n, --name=NAME The name of the S3 logging object. Used as a
Expand All @@ -769,6 +769,7 @@ COMMANDS
--bucket=BUCKET Your S3 bucket name
--access-key=ACCESS-KEY Your S3 account access key
--secret-key=SECRET-KEY Your S3 account secret key
--iam-role=IAM-ROLE The IAM role ARN for logging
-s, --service-id=SERVICE-ID Service ID
--domain=DOMAIN The domain of the S3 endpoint
--path=PATH The path to upload logs to
Expand Down Expand Up @@ -831,6 +832,7 @@ COMMANDS
--bucket=BUCKET Your S3 bucket name
--access-key=ACCESS-KEY Your S3 account access key
--secret-key=SECRET-KEY Your S3 account secret key
--iam-role=IAM-ROLE The IAM role ARN for logging
--domain=DOMAIN The domain of the S3 endpoint
--path=PATH The path to upload logs to
--period=PERIOD How frequently log files are finalized so they
Expand Down
48 changes: 42 additions & 6 deletions pkg/logging/s3/create.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package s3

import (
"fmt"
"io"

"github.com/fastly/cli/pkg/common"
Expand All @@ -20,8 +21,12 @@ type CreateCommand struct {
EndpointName string // Can't shadow common.Base method Name().
Version int
BucketName string
AccessKey string
SecretKey string

// mutual exclusions
// AccessKey + SecretKey or IAMRole must be provided
AccessKey common.OptionalString
SecretKey common.OptionalString
IAMRole common.OptionalString

// optional
Domain common.OptionalString
Expand Down Expand Up @@ -51,8 +56,10 @@ func NewCreateCommand(parent common.Registerer, globals *config.Data) *CreateCom
c.CmdClause.Flag("name", "The name of the S3 logging object. Used as a primary key for API access").Short('n').Required().StringVar(&c.EndpointName)
c.CmdClause.Flag("version", "Number of service version").Required().IntVar(&c.Version)
c.CmdClause.Flag("bucket", "Your S3 bucket name").Required().StringVar(&c.BucketName)
c.CmdClause.Flag("access-key", "Your S3 account access key").Required().StringVar(&c.AccessKey)
c.CmdClause.Flag("secret-key", "Your S3 account secret key").Required().StringVar(&c.SecretKey)

c.CmdClause.Flag("access-key", "Your S3 account access key").Action(c.AccessKey.Set).StringVar(&c.AccessKey.Value)
c.CmdClause.Flag("secret-key", "Your S3 account secret key").Action(c.SecretKey.Set).StringVar(&c.SecretKey.Value)
c.CmdClause.Flag("iam-role", "The IAM role ARN for logging").Action(c.IAMRole.Set).StringVar(&c.IAMRole.Value)

c.CmdClause.Flag("service-id", "Service ID").Short('s').StringVar(&c.manifest.Flag.ServiceID)
c.CmdClause.Flag("domain", "The domain of the S3 endpoint").Action(c.Domain.Set).StringVar(&c.Domain.Value)
Expand Down Expand Up @@ -86,8 +93,37 @@ func (c *CreateCommand) createInput() (*fastly.CreateS3Input, error) {
input.ServiceVersion = c.Version
input.Name = c.EndpointName
input.BucketName = c.BucketName
input.AccessKey = c.AccessKey
input.SecretKey = c.SecretKey

// The following block checks for invalid permutations of the ways in
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was looking for something akin to the ArgGroup feature in the clap library for rust, but I didn't see anything similar present in kingpin so I just implemented the checks manually here. If this ever became a more common pattern we could try to add it into the kingpin library or just our fork of it since it's unmaintained because it'd be a lot nicer to have it handled inside the library versus a bunch of ad hoc enforcement in random places in our code.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed, if this becomes more common then we can think about implementing it within the Fastly Kingpin fork.

// which the AccessKey + SecretKey and IAMRole flags can be
// provided. This is necessary because either the AccessKey and
// SecretKey or the IAMRole is required, but they are mutually
// exclusive. The kingpin library lacks a way to express this constraint
// via the flag specification API so we enforce it manually here.
if !c.AccessKey.WasSet && !c.SecretKey.WasSet && !c.IAMRole.WasSet {
return nil, fmt.Errorf("error parsing arguments: the --access-key and --secret-key flags or the --iam-role flag must be provided")
} else if (c.AccessKey.WasSet || c.SecretKey.WasSet) && c.IAMRole.WasSet {
// Enforce mutual exclusion
return nil, fmt.Errorf("error parsing arguments: the --access-key and --secret-key flags are mutually exclusive with the --iam-role flag")
} else if c.AccessKey.WasSet && !c.SecretKey.WasSet {
return nil, fmt.Errorf("error parsing arguments: required flag --secret-key not provided")

} else if !c.AccessKey.WasSet && c.SecretKey.WasSet {
return nil, fmt.Errorf("error parsing arguments: required flag --access-key not provided")

}

if c.AccessKey.WasSet {
input.AccessKey = c.AccessKey.Value
}

if c.SecretKey.WasSet {
input.SecretKey = c.SecretKey.Value
}

if c.IAMRole.WasSet {
input.IAMRole = c.IAMRole.Value
}

if c.Domain.WasSet {
input.Domain = c.Domain.Value
Expand Down
9 changes: 7 additions & 2 deletions pkg/logging/s3/describe.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,13 @@ func (c *DescribeCommand) Exec(in io.Reader, out io.Writer) error {
fmt.Fprintf(out, "Version: %d\n", s3.ServiceVersion)
fmt.Fprintf(out, "Name: %s\n", s3.Name)
fmt.Fprintf(out, "Bucket: %s\n", s3.BucketName)
fmt.Fprintf(out, "Access key: %s\n", s3.AccessKey)
fmt.Fprintf(out, "Secret key: %s\n", s3.SecretKey)
if s3.AccessKey != "" || s3.SecretKey != "" {
fmt.Fprintf(out, "Access key: %s\n", s3.AccessKey)
fmt.Fprintf(out, "Secret key: %s\n", s3.SecretKey)
}
if s3.IAMRole != "" {
fmt.Fprintf(out, "IAM role: %s\n", s3.IAMRole)
}
fmt.Fprintf(out, "Path: %s\n", s3.Path)
fmt.Fprintf(out, "Period: %d\n", s3.Period)
fmt.Fprintf(out, "GZip level: %d\n", s3.GzipLevel)
Expand Down
9 changes: 7 additions & 2 deletions pkg/logging/s3/list.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,8 +62,13 @@ func (c *ListCommand) Exec(in io.Reader, out io.Writer) error {
fmt.Fprintf(out, "\t\tVersion: %d\n", s3.ServiceVersion)
fmt.Fprintf(out, "\t\tName: %s\n", s3.Name)
fmt.Fprintf(out, "\t\tBucket: %s\n", s3.BucketName)
fmt.Fprintf(out, "\t\tAccess key: %s\n", s3.AccessKey)
fmt.Fprintf(out, "\t\tSecret key: %s\n", s3.SecretKey)
if s3.AccessKey != "" || s3.SecretKey != "" {
fmt.Fprintf(out, "\t\tAccess key: %s\n", s3.AccessKey)
fmt.Fprintf(out, "\t\tSecret key: %s\n", s3.SecretKey)
}
if s3.IAMRole != "" {
fmt.Fprintf(out, "\t\tIAM role: %s\n", s3.IAMRole)
}
fmt.Fprintf(out, "\t\tPath: %s\n", s3.Path)
fmt.Fprintf(out, "\t\tPeriod: %d\n", s3.Period)
fmt.Fprintf(out, "\t\tGZip level: %d\n", s3.GzipLevel)
Expand Down
36 changes: 36 additions & 0 deletions pkg/logging/s3/s3_integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,30 @@ func TestS3Create(t *testing.T) {
wantError string
wantOutput string
}{
{
args: []string{"logging", "s3", "create", "--service-id", "123", "--version", "1", "--name", "log", "--bucket", "log"},
wantError: "error parsing arguments: the --access-key and --secret-key flags or the --iam-role flag must be provided",
},
{
args: []string{"logging", "s3", "create", "--service-id", "123", "--version", "1", "--name", "log", "--bucket", "log", "--access-key", "foo"},
wantError: "error parsing arguments: required flag --secret-key not provided",
},
{
args: []string{"logging", "s3", "create", "--service-id", "123", "--version", "1", "--name", "log", "--bucket", "log", "--secret-key", "bar"},
wantError: "error parsing arguments: required flag --access-key not provided",
},
{
args: []string{"logging", "s3", "create", "--service-id", "123", "--version", "1", "--name", "log", "--bucket", "log", "--secret-key", "bar", "--iam-role", "arn:aws:iam::123456789012:role/S3Access"},
wantError: "error parsing arguments: the --access-key and --secret-key flags are mutually exclusive with the --iam-role flag",
},
{
args: []string{"logging", "s3", "create", "--service-id", "123", "--version", "1", "--name", "log", "--bucket", "log", "--access-key", "foo", "--iam-role", "arn:aws:iam::123456789012:role/S3Access"},
wantError: "error parsing arguments: the --access-key and --secret-key flags are mutually exclusive with the --iam-role flag",
},
{
args: []string{"logging", "s3", "create", "--service-id", "123", "--version", "1", "--name", "log", "--bucket", "log", "--access-key", "foo", "--secret-key", "bar", "--iam-role", "arn:aws:iam::123456789012:role/S3Access"},
wantError: "error parsing arguments: the --access-key and --secret-key flags are mutually exclusive with the --iam-role flag",
},
{
args: []string{"logging", "s3", "create", "--service-id", "123", "--version", "1", "--name", "log", "--bucket", "log", "--access-key", "foo", "--secret-key", "bar"},
api: mock.API{CreateS3Fn: createS3OK},
Expand All @@ -37,6 +57,16 @@ func TestS3Create(t *testing.T) {
api: mock.API{CreateS3Fn: createS3Error},
wantError: errTest.Error(),
},
{
args: []string{"logging", "s3", "create", "--service-id", "123", "--version", "1", "--name", "log2", "--bucket", "log", "--iam-role", "arn:aws:iam::123456789012:role/S3Access"},
api: mock.API{CreateS3Fn: createS3OK},
wantOutput: "Created S3 logging endpoint log2 (service 123 version 1)",
},
{
args: []string{"logging", "s3", "create", "--service-id", "123", "--version", "1", "--name", "log2", "--bucket", "log", "--iam-role", "arn:aws:iam::123456789012:role/S3Access"},
api: mock.API{CreateS3Fn: createS3Error},
wantError: errTest.Error(),
},
} {
t.Run(strings.Join(testcase.args, " "), func(t *testing.T) {
var (
Expand Down Expand Up @@ -176,6 +206,11 @@ func TestS3Update(t *testing.T) {
api: mock.API{UpdateS3Fn: updateS3OK},
wantOutput: "Updated S3 logging endpoint log (service 123 version 1)",
},
{
args: []string{"logging", "s3", "update", "--service-id", "123", "--version", "1", "--name", "logs", "--access-key", "foo", "--secret-key", "bar", "--iam-role", ""},
api: mock.API{UpdateS3Fn: updateS3OK},
wantOutput: "Updated S3 logging endpoint log (service 123 version 1)",
},
} {
t.Run(strings.Join(testcase.args, " "), func(t *testing.T) {
var (
Expand Down Expand Up @@ -260,6 +295,7 @@ func listS3sOK(i *fastly.ListS3sInput) ([]*fastly.S3, error) {
BucketName: "my-logs",
AccessKey: "1234",
SecretKey: "-----BEGIN RSA PRIVATE KEY-----MIIEogIBAAKCA",
IAMRole: "",
Domain: "https://s3.us-east-1.amazonaws.com",
Path: "logs/",
Period: 3600,
Expand Down
33 changes: 28 additions & 5 deletions pkg/logging/s3/s3_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ func TestCreateS3Input(t *testing.T) {
wantError string
}{
{
name: "required values set flag serviceID",
name: "required values set flag serviceID using access credentials",
cmd: createCommandRequired(),
want: &fastly.CreateS3Input{
ServiceID: "123",
Expand All @@ -32,6 +32,17 @@ func TestCreateS3Input(t *testing.T) {
SecretKey: "secret",
},
},
{
name: "required values set flag serviceID using IAM role",
cmd: createCommandRequiredIAMRole(),
want: &fastly.CreateS3Input{
ServiceID: "123",
ServiceVersion: 2,
Name: "log",
BucketName: "bucket",
IAMRole: "arn:aws:iam::123456789012:role/S3Access",
},
},
{
name: "all values set flag serviceID",
cmd: createCommandAll(),
Expand Down Expand Up @@ -103,6 +114,7 @@ func TestUpdateS3Input(t *testing.T) {
BucketName: fastly.String("new2"),
AccessKey: fastly.String("new3"),
SecretKey: fastly.String("new4"),
IAMRole: fastly.String(""),
Domain: fastly.String("new5"),
Path: fastly.String("new6"),
Period: fastly.Uint(3601),
Expand Down Expand Up @@ -142,8 +154,18 @@ func createCommandRequired() *CreateCommand {
EndpointName: "log",
Version: 2,
BucketName: "bucket",
AccessKey: "access",
SecretKey: "secret",
AccessKey: common.OptionalString{Optional: common.Optional{WasSet: true}, Value: "access"},
SecretKey: common.OptionalString{Optional: common.Optional{WasSet: true}, Value: "secret"},
}
}

func createCommandRequiredIAMRole() *CreateCommand {
return &CreateCommand{
manifest: manifest.Data{Flag: manifest.Flag{ServiceID: "123"}},
EndpointName: "log",
Version: 2,
BucketName: "bucket",
IAMRole: common.OptionalString{Optional: common.Optional{WasSet: true}, Value: "arn:aws:iam::123456789012:role/S3Access"},
}
}

Expand All @@ -153,8 +175,8 @@ func createCommandAll() *CreateCommand {
EndpointName: "logs",
Version: 2,
BucketName: "bucket",
AccessKey: "access",
SecretKey: "secret",
AccessKey: common.OptionalString{Optional: common.Optional{WasSet: true}, Value: "access"},
SecretKey: common.OptionalString{Optional: common.Optional{WasSet: true}, Value: "secret"},
Domain: common.OptionalString{Optional: common.Optional{WasSet: true}, Value: "domain"},
Path: common.OptionalString{Optional: common.Optional{WasSet: true}, Value: "path"},
Period: common.OptionalUint{Optional: common.Optional{WasSet: true}, Value: 3600},
Expand Down Expand Up @@ -197,6 +219,7 @@ func updateCommandAll() *UpdateCommand {
BucketName: common.OptionalString{Optional: common.Optional{WasSet: true}, Value: "new2"},
AccessKey: common.OptionalString{Optional: common.Optional{WasSet: true}, Value: "new3"},
SecretKey: common.OptionalString{Optional: common.Optional{WasSet: true}, Value: "new4"},
IAMRole: common.OptionalString{Optional: common.Optional{WasSet: true}, Value: ""},
Domain: common.OptionalString{Optional: common.Optional{WasSet: true}, Value: "new5"},
Path: common.OptionalString{Optional: common.Optional{WasSet: true}, Value: "new6"},
Period: common.OptionalUint{Optional: common.Optional{WasSet: true}, Value: 3601},
Expand Down
6 changes: 6 additions & 0 deletions pkg/logging/s3/update.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ type UpdateCommand struct {
BucketName common.OptionalString
AccessKey common.OptionalString
SecretKey common.OptionalString
IAMRole common.OptionalString
Domain common.OptionalString
Path common.OptionalString
Period common.OptionalUint
Expand Down Expand Up @@ -59,6 +60,7 @@ func NewUpdateCommand(parent common.Registerer, globals *config.Data) *UpdateCom
c.CmdClause.Flag("bucket", "Your S3 bucket name").Action(c.BucketName.Set).StringVar(&c.BucketName.Value)
c.CmdClause.Flag("access-key", "Your S3 account access key").Action(c.AccessKey.Set).StringVar(&c.AccessKey.Value)
c.CmdClause.Flag("secret-key", "Your S3 account secret key").Action(c.SecretKey.Set).StringVar(&c.SecretKey.Value)
c.CmdClause.Flag("iam-role", "The IAM role ARN for logging").Action(c.IAMRole.Set).StringVar(&c.IAMRole.Value)
c.CmdClause.Flag("domain", "The domain of the S3 endpoint").Action(c.Domain.Set).StringVar(&c.Domain.Value)
c.CmdClause.Flag("path", "The path to upload logs to").Action(c.Path.Set).StringVar(&c.Path.Value)
c.CmdClause.Flag("period", "How frequently log files are finalized so they can be available for reading (in seconds, default 3600)").Action(c.Period.Set).UintVar(&c.Period.Value)
Expand Down Expand Up @@ -106,6 +108,10 @@ func (c *UpdateCommand) createInput() (*fastly.UpdateS3Input, error) {
input.SecretKey = fastly.String(c.SecretKey.Value)
}

if c.IAMRole.WasSet {
input.IAMRole = fastly.String(c.IAMRole.Value)
}

if c.Domain.WasSet {
input.Domain = fastly.String(c.Domain.Value)
}
Expand Down
0