-
Notifications
You must be signed in to change notification settings - Fork 69
Add support for Kinesis logging endpoint #177
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
Changes from all commits
75eccff
944cf3d
cacda3a
8bbe7a1
42e6b99
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,109 @@ | ||
package kinesis | ||
|
||
import ( | ||
"io" | ||
|
||
"github.com/fastly/cli/pkg/common" | ||
"github.com/fastly/cli/pkg/compute/manifest" | ||
"github.com/fastly/cli/pkg/config" | ||
"github.com/fastly/cli/pkg/errors" | ||
"github.com/fastly/cli/pkg/text" | ||
"github.com/fastly/go-fastly/v2/fastly" | ||
) | ||
|
||
// CreateCommand calls the Fastly API to create an Amazon Kinesis logging endpoint. | ||
type CreateCommand struct { | ||
common.Base | ||
manifest manifest.Data | ||
|
||
// required | ||
EndpointName string // Can't shadow common.Base method Name(). | ||
Version int | ||
StreamName string | ||
AccessKey string | ||
SecretKey string | ||
Region string | ||
|
||
// optional | ||
Format common.OptionalString | ||
FormatVersion common.OptionalUint | ||
ResponseCondition common.OptionalString | ||
Placement common.OptionalString | ||
} | ||
|
||
// NewCreateCommand returns a usable command registered under the parent. | ||
func NewCreateCommand(parent common.Registerer, globals *config.Data) *CreateCommand { | ||
var c CreateCommand | ||
c.Globals = globals | ||
c.manifest.File.Read(manifest.Filename) | ||
c.CmdClause = parent.Command("create", "Create an Amazon Kinesis logging endpoint on a Fastly service version").Alias("add") | ||
|
||
// required | ||
c.CmdClause.Flag("name", "The name of the Kinesis 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("stream-name", "The Amazon Kinesis stream to send logs to").Required().StringVar(&c.StreamName) | ||
c.CmdClause.Flag("access-key", "The access key associated with the target Amazon Kinesis stream").Required().StringVar(&c.AccessKey) | ||
c.CmdClause.Flag("secret-key", "The secret key associated with the target Amazon Kinesis stream").Required().StringVar(&c.SecretKey) | ||
c.CmdClause.Flag("region", "The AWS region where the Kinesis stream exists").Required().StringVar(&c.Region) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Are we sure 'region' is required? It doesn't look to be in the API docs example (nor in Go-Fastly). There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, it is required. The configuration validation will fail if it is not present. I see what you mean in the case of There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Question:
I presume 'config validation' refers to the upstream API's own validation, would that be correct? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I see Go-Fastly currently doesn't define validation for the 'region' field, but if we're saying the API itself is expecting a region field otherwise it'll return an error, then we should ensure a validation check for the 'region' field is added into Go-Fastly (I have an existing PR open I can add this to). But for the time being, this is fine to be marked as
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Also, just FYI (most of which will be obvious to you): my perspective on this is that we have three tools at play here:
We should be able to trust the API to always have the right validation in place, but (as you know) by having validation outside of the API it means we can prevent slowing down a client trying to carry out operations that otherwise we know are just going to fail by the time they reach the API layer, so short-circuiting those requests is better for the client as far as the 'feedback loop' is concerned but also good for the API because it doesn't have to handle the request in the first place. Considering Go-Fastly and CLI sit at effectively the same level (i.e. they exist on the client) we need to decide which 'tool' is responsible for handling validation of obvious issues with a client request, and for me that should be Go-Fastly as it's the thing that's ultimately fronting the API. So my hope is that with the work @doramatadora has been doing on the new OpenAPI specification it will mean we can move to a place where we more concretely know what is a required field and have Go-Fastly handle validation and thus reduce duplicating logic in the CLI (although admittedly at this point in time the OpenAPI spec for Kinesis logging still doesn't indicate that 'region' is a required field nor does it show it as part of its example code so that would likely need updating). There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
That's correct.
If you send me the service id in slack I can enable this for you. I'll also follow-up on the docs side of things and open a change request if needed to have it indicate |
||
|
||
// optional | ||
c.CmdClause.Flag("service-id", "Service ID").Short('s').StringVar(&c.manifest.Flag.ServiceID) | ||
c.CmdClause.Flag("format", "Apache style log formatting").Action(c.Format.Set).StringVar(&c.Format.Value) | ||
c.CmdClause.Flag("format-version", "The version of the custom logging format used for the configured endpoint. Can be either 2 (default) or 1").Action(c.FormatVersion.Set).UintVar(&c.FormatVersion.Value) | ||
c.CmdClause.Flag("response-condition", "The name of an existing condition in the configured endpoint, or leave blank to always execute").Action(c.ResponseCondition.Set).StringVar(&c.ResponseCondition.Value) | ||
c.CmdClause.Flag("placement", "Where in the generated VCL the logging call should be placed, overriding any format_version default. Can be none or waf_debug").Action(c.Placement.Set).StringVar(&c.Placement.Value) | ||
|
||
return &c | ||
} | ||
|
||
// createInput transforms values parsed from CLI flags into an object to be used by the API client library. | ||
func (c *CreateCommand) createInput() (*fastly.CreateKinesisInput, error) { | ||
var input fastly.CreateKinesisInput | ||
|
||
serviceID, source := c.manifest.ServiceID() | ||
if source == manifest.SourceUndefined { | ||
return nil, errors.ErrNoServiceID | ||
} | ||
|
||
input.ServiceID = serviceID | ||
input.ServiceVersion = c.Version | ||
input.Name = c.EndpointName | ||
input.StreamName = c.StreamName | ||
input.AccessKey = c.AccessKey | ||
input.SecretKey = c.SecretKey | ||
input.Region = c.Region | ||
|
||
if c.Format.WasSet { | ||
input.Format = c.Format.Value | ||
} | ||
|
||
if c.FormatVersion.WasSet { | ||
input.FormatVersion = c.FormatVersion.Value | ||
} | ||
|
||
if c.ResponseCondition.WasSet { | ||
input.ResponseCondition = c.ResponseCondition.Value | ||
} | ||
|
||
if c.Placement.WasSet { | ||
input.Placement = c.Placement.Value | ||
} | ||
|
||
return &input, nil | ||
} | ||
|
||
// Exec invokes the application logic for the command. | ||
func (c *CreateCommand) Exec(in io.Reader, out io.Writer) error { | ||
input, err := c.createInput() | ||
if err != nil { | ||
return err | ||
} | ||
|
||
d, err := c.Globals.Client.CreateKinesis(input) | ||
if err != nil { | ||
return err | ||
} | ||
|
||
text.Success(out, "Created Kinesis logging endpoint %s (service %s version %d)", d.Name, d.ServiceID, d.ServiceVersion) | ||
return nil | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,48 @@ | ||
package kinesis | ||
|
||
import ( | ||
"io" | ||
|
||
"github.com/fastly/cli/pkg/common" | ||
"github.com/fastly/cli/pkg/compute/manifest" | ||
"github.com/fastly/cli/pkg/config" | ||
"github.com/fastly/cli/pkg/errors" | ||
"github.com/fastly/cli/pkg/text" | ||
"github.com/fastly/go-fastly/v2/fastly" | ||
) | ||
|
||
// DeleteCommand calls the Fastly API to delete an Amazon Kinesis logging endpoint. | ||
type DeleteCommand struct { | ||
common.Base | ||
manifest manifest.Data | ||
Input fastly.DeleteKinesisInput | ||
} | ||
|
||
// NewDeleteCommand returns a usable command registered under the parent. | ||
func NewDeleteCommand(parent common.Registerer, globals *config.Data) *DeleteCommand { | ||
var c DeleteCommand | ||
c.Globals = globals | ||
c.manifest.File.Read(manifest.Filename) | ||
c.CmdClause = parent.Command("delete", "Delete a Kinesis logging endpoint on a Fastly service version").Alias("remove") | ||
c.CmdClause.Flag("version", "Number of service version").Required().IntVar(&c.Input.ServiceVersion) | ||
c.CmdClause.Flag("name", "The name of the Kinesis logging object").Short('n').Required().StringVar(&c.Input.Name) | ||
c.CmdClause.Flag("service-id", "Service ID").Short('s').StringVar(&c.manifest.Flag.ServiceID) | ||
|
||
return &c | ||
} | ||
|
||
// Exec invokes the application logic for the command. | ||
func (c *DeleteCommand) Exec(in io.Reader, out io.Writer) error { | ||
serviceID, source := c.manifest.ServiceID() | ||
if source == manifest.SourceUndefined { | ||
return errors.ErrNoServiceID | ||
} | ||
c.Input.ServiceID = serviceID | ||
|
||
if err := c.Globals.Client.DeleteKinesis(&c.Input); err != nil { | ||
return err | ||
} | ||
|
||
text.Success(out, "Deleted Kinesis logging endpoint %s (service %s version %d)", c.Input.Name, c.Input.ServiceID, c.Input.ServiceVersion) | ||
return nil | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,59 @@ | ||
package kinesis | ||
|
||
import ( | ||
"fmt" | ||
"io" | ||
|
||
"github.com/fastly/cli/pkg/common" | ||
"github.com/fastly/cli/pkg/compute/manifest" | ||
"github.com/fastly/cli/pkg/config" | ||
"github.com/fastly/cli/pkg/errors" | ||
"github.com/fastly/go-fastly/v2/fastly" | ||
) | ||
|
||
// DescribeCommand calls the Fastly API to describe an Amazon Kinesis logging endpoint. | ||
type DescribeCommand struct { | ||
common.Base | ||
manifest manifest.Data | ||
Input fastly.GetKinesisInput | ||
} | ||
|
||
// NewDescribeCommand returns a usable command registered under the parent. | ||
func NewDescribeCommand(parent common.Registerer, globals *config.Data) *DescribeCommand { | ||
var c DescribeCommand | ||
c.Globals = globals | ||
c.manifest.File.Read(manifest.Filename) | ||
c.CmdClause = parent.Command("describe", "Show detailed information about a Kinesis logging endpoint on a Fastly service version").Alias("get") | ||
c.CmdClause.Flag("service-id", "Service ID").Short('s').StringVar(&c.manifest.Flag.ServiceID) | ||
c.CmdClause.Flag("version", "Number of service version").Required().IntVar(&c.Input.ServiceVersion) | ||
c.CmdClause.Flag("name", "The name of the Kinesis logging object").Short('n').Required().StringVar(&c.Input.Name) | ||
return &c | ||
} | ||
|
||
// Exec invokes the application logic for the command. | ||
func (c *DescribeCommand) Exec(in io.Reader, out io.Writer) error { | ||
serviceID, source := c.manifest.ServiceID() | ||
if source == manifest.SourceUndefined { | ||
return errors.ErrNoServiceID | ||
} | ||
c.Input.ServiceID = serviceID | ||
|
||
kinesis, err := c.Globals.Client.GetKinesis(&c.Input) | ||
if err != nil { | ||
return err | ||
} | ||
|
||
fmt.Fprintf(out, "Service ID: %s\n", kinesis.ServiceID) | ||
fmt.Fprintf(out, "Version: %d\n", kinesis.ServiceVersion) | ||
fmt.Fprintf(out, "Name: %s\n", kinesis.Name) | ||
fmt.Fprintf(out, "Stream name: %s\n", kinesis.StreamName) | ||
fmt.Fprintf(out, "Region: %s\n", kinesis.Region) | ||
fmt.Fprintf(out, "Access key: %s\n", kinesis.AccessKey) | ||
fmt.Fprintf(out, "Secret key: %s\n", kinesis.SecretKey) | ||
fmt.Fprintf(out, "Format: %s\n", kinesis.Format) | ||
fmt.Fprintf(out, "Format version: %d\n", kinesis.FormatVersion) | ||
fmt.Fprintf(out, "Response condition: %s\n", kinesis.ResponseCondition) | ||
fmt.Fprintf(out, "Placement: %s\n", kinesis.Placement) | ||
Comment on lines
+46
to
+56
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we should create a new Something generic like this would suffice: https://play.golang.org/p/csgKDzLZPeu Which means you could do something like: import "github.com/fastly/cli/pkg/text"
text.PrintLines(out, text.Lines{
"Service ID": kinesis.ServiceID,
"Version": kinesis.ServiceVersion,
"Name": kinesis.Name,
...
}) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @phamann what do you think about this... If we combined this generic implementation (see my comment above) with You could even take it a step further and add in the ability to pass in an optional callback function for those edge cases such as There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 👍 to this idea from me. I'd say this is probably out of scope for this PR, but I think it would be a good change to implement broadly. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Agree on both accounts:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 👍🏻 👌🏻 |
||
|
||
return nil | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
// Package kinesis contains commands to inspect and manipulate Fastly service Kinesis | ||
// logging endpoints. | ||
package kinesis |
Uh oh!
There was an error while loading. Please reload this page.