Skip to content

Commit

Permalink
Improve synthetics and cloud code (#104)
Browse files Browse the repository at this point in the history
This commit includes improvements made while working on Synthetics Tests
interface:
- For existing cloud/synthetics code, move errors conversion up to API
  layer (from payload layer) to simplify the code and prevent omitting
  the conversion. Alongside that some error conversion bugs in cloud
  exports code were fixed.
- Remove Alibaba provider from cloud.Provider enum as it is not
  supported
- Generate String() method for Code type to fix its printing
- Describe stringer tool installation and code generation in readme.md
- Modify Start() function of fake servers to prevent flakiness
- Move spySyntheticsServer to separate file, because it is also going to
  be used in synth_test_integration_test.go
- Cosmetic improvements made while reading the code

Issue: KNTK-399
  • Loading branch information
Daniel Furman authored Jun 24, 2022
1 parent 13fe29b commit d23c437
Show file tree
Hide file tree
Showing 19 changed files with 373 additions and 284 deletions.
5 changes: 5 additions & 0 deletions .golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,11 @@ linters-settings:
default-signifies-exhaustive: true
gocyclo:
min-complexity: 10
gomnd:
settings:
mnd:
# 10 is usually obvious, e.g. strconv.FormatInt(n, 10)
ignored-numbers: "10"
govet:
disable:
# Reordering struct fields may decrease readability
Expand Down
4 changes: 3 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -46,12 +46,14 @@ Library provides a set of [functions](./kentikapi/errors.go) to check error type
Anybody who wants to contribute to development is welcome to provide pull requests. To work on the SDK, install tools listed in [requirements section](#requirements).

Optional tools:
- _golangci-lint_: <https://golangci-lint.run/usage/install/#local-installation>
- _golangci-lint_ for static code analysis: <https://golangci-lint.run/usage/install/#local-installation>
- _stringer_ for generating enum String() methods: `go install golang.org/x/tools/cmd/stringer`

Development steps:
- Compile the code: `go build -tags examples ./...`
- Run tests: `go test ./...`
- Run all tests, including usage examples: `go test -tags examples ./...`
- Regenerate the code: `go generate ./...`
- Run golangci-lint: `golangci-lint run ./...`
- Format the code: `./tools/fmt.sh`
- Check Go module consistency: `./tools/check-go-mod.sh`
Expand Down
2 changes: 1 addition & 1 deletion examples/cloud_export_example_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ func demonstrateCloudAPIWithAWSExport() error {
PrettyPrint(ce)

fmt.Println("### Updating AWS cloud export")
ce.Name = "go-sdk-updated-aws-export"
ce.Name = "go-sdk-example-updated-aws-export"
ce.Description = "Updated description"
ce.GetAWSProperties().Bucket = "updated-bucket"
ce.BGP.UseBGPDeviceID = "updated-bgp-device-id"
Expand Down
8 changes: 4 additions & 4 deletions examples/synthetics_example_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,9 @@ func TestDemonstrateSyntheticsAgentsAPI(t *testing.T) {
assert.NoError(t, err)
}

func TestDemonstrateSyntheticsTestAPI(t *testing.T) {
func TestDemonstrateSyntheticsTestsAPI(t *testing.T) {
t.Parallel()
err := demonstrateSyntheticsTestAPI()
err := demonstrateSyntheticsTestsAPI()
assert.NoError(t, err)
}

Expand Down Expand Up @@ -71,7 +71,7 @@ func demonstrateSyntheticsAgentAPI() error {

fmt.Println("### Updating synthetic agent")
originalAlias := agent.Alias
agent.Alias = "go-sdk-updated-alias"
agent.Alias = "go-sdk-example-updated-alias"

agent, err = client.Synthetics.UpdateAgent(ctx, agent)
if err != nil {
Expand Down Expand Up @@ -135,7 +135,7 @@ func pickPrivateAgentID(ctx context.Context) (string, error) {
return "", fmt.Errorf("no private agent found: %w", err)
}

func demonstrateSyntheticsTestAPI() error {
func demonstrateSyntheticsTestsAPI() error {
ctx := context.Background()
client, err := NewClient()
if err != nil {
Expand Down
1 change: 1 addition & 0 deletions examples/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ func NewClient() (*kentikapi.Client, error) {

return kentikapi.NewClient(
kentikapi.WithCredentials(email, token),
kentikapi.WithLogPayloads(),
)
}

Expand Down
4 changes: 2 additions & 2 deletions go.mod

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

8 changes: 6 additions & 2 deletions go.sum

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 4 additions & 1 deletion kentikapi/client_integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package kentikapi_test

import (
"context"
"errors"
"fmt"
"net"
"net/http"
Expand Down Expand Up @@ -384,7 +385,9 @@ func (s *spySyntheticsServerWithDelays) Start() {

go func() {
err = s.server.Serve(l)
assert.NoError(s.t, err)
if !errors.Is(err, grpc.ErrServerStopped) {
assert.NoError(s.t, err)
}
s.done <- struct{}{}
}()
}
Expand Down
25 changes: 12 additions & 13 deletions kentikapi/cloud/export.go
Original file line number Diff line number Diff line change
Expand Up @@ -186,54 +186,54 @@ type ExportStatus struct {
StorageAccountAccess *bool
}

// AWSExportRequiredFields is subset of fields required to create an AWS Export.
// AWSExportRequiredFields is a subset of fields required to create an AWS Export.
type AWSExportRequiredFields struct {
Name string
PlanID string
AWSProperties AWSPropertiesRequiredFields
}

// AWSPropertiesRequiredFields is subset of AWSProperties required to create an AWS Export.
// AWSPropertiesRequiredFields is a subset of AWSProperties required to create an AWS Export.
type AWSPropertiesRequiredFields struct {
Bucket string
}

// AzureExportRequiredFields is subset of fields required to create an Azure Export.
// AzureExportRequiredFields is a subset of fields required to create an Azure Export.
type AzureExportRequiredFields struct {
Name string
PlanID string
AzureProperties AzurePropertiesRequiredFields
}

// AzurePropertiesRequiredFields is subset of AzureProperties required to create an Azure Export.
// AzurePropertiesRequiredFields is a subset of AzureProperties required to create an Azure Export.
type AzurePropertiesRequiredFields struct {
Location string
ResourceGroup string
StorageAccount string
SubscriptionID string
}

// GCEExportRequiredFields is subset of fields required to create a GCE Export.
// GCEExportRequiredFields is a subset of fields required to create a GCE Export.
type GCEExportRequiredFields struct {
Name string
PlanID string
GCEProperties GCEPropertiesRequiredFields
}

// GCEPropertiesRequiredFields is subset of GCEProperties required to create a GCE Export.
// GCEPropertiesRequiredFields is a subset of GCEProperties required to create a GCE Export.
type GCEPropertiesRequiredFields struct {
Project string
Subscription string
}

// IBMExportRequiredFields is subset of fields required to create an IBM Export.
// IBMExportRequiredFields is a subset of fields required to create an IBM Export.
type IBMExportRequiredFields struct {
Name string
PlanID string
IBMProperties IBMPropertiesRequiredFields
}

// IBMPropertiesRequiredFields is subset of IBMProperties required to create an IBM Export.
// IBMPropertiesRequiredFields is a subset of IBMProperties required to create an IBM Export.
type IBMPropertiesRequiredFields struct {
Bucket string
}
Expand All @@ -255,9 +255,8 @@ const (
type Provider string

const (
ProviderAlibaba Provider = "alibaba"
ProviderAWS Provider = "aws"
ProviderAzure Provider = "azure"
ProviderGCE Provider = "gce" // gcp value in Agents API
ProviderIBM Provider = "ibm"
ProviderAWS Provider = "aws"
ProviderAzure Provider = "azure"
ProviderGCE Provider = "gce" // gcp value in Agents API
ProviderIBM Provider = "ibm"
)
42 changes: 20 additions & 22 deletions kentikapi/cloud_exports_integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ func TestClient_Cloud_GetAllExports(t *testing.T) {
InvalidExportsCount: 1,
},
}, {
name: "2 exports received - one empty",
name: "2 exports received - one nil",
response: listCEResponse{
data: &cloudexportpb.ListCloudExportResponse{
Exports: []*cloudexportpb.CloudExport{
Expand All @@ -97,8 +97,7 @@ func TestClient_Cloud_GetAllExports(t *testing.T) {
InvalidExportsCount: 0,
},
},
// empty response fails validation
expectedError: true,
expectedError: true, // InvalidResponse
},
}
//nolint:dupl
Expand Down Expand Up @@ -178,7 +177,7 @@ func TestClient_Cloud_GetExport(t *testing.T) {
response: getCEResponse{
data: &cloudexportpb.GetCloudExportResponse{},
},
expectedError: true,
expectedError: true, // InvalidResponse
}, {
name: "minimal AWS cloud export received",
requestID: "58192",
Expand Down Expand Up @@ -341,7 +340,7 @@ func TestClient_Cloud_CreateExport(t *testing.T) {
data: &cloudexportpb.CreateCloudExportResponse{Export: nil},
},
expectedResult: nil,
expectedError: true,
expectedError: true, // InvalidResponse
}, {
name: "minimal AWS export created",
request: cloud.NewAWSExport(cloud.AWSExportRequiredFields{
Expand Down Expand Up @@ -481,7 +480,7 @@ func TestClient_Cloud_CreateExport(t *testing.T) {
},
expectedResult: newIBMExport(),
}, {
name: "create request validation, missing AWS.BUCKET",
name: "AWS export with missing AWSProperties.Bucket field",
request: cloud.NewAWSExport(cloud.AWSExportRequiredFields{
Name: "invalid-aws-export",
PlanID: "11467",
Expand Down Expand Up @@ -513,7 +512,7 @@ func TestClient_Cloud_CreateExport(t *testing.T) {
result, err := client.Cloud.CreateExport(context.Background(), tt.request)

// assert
t.Logf("Got err: %v", err)
t.Logf("Got result: %+v, err: %v", result, err)
if tt.expectedError {
assert.Error(t, err)
for _, isErr := range tt.errorPredicates {
Expand All @@ -523,7 +522,9 @@ func TestClient_Cloud_CreateExport(t *testing.T) {
assert.NoError(t, err)
}

if tt.expectedRequest != nil && assert.Equal(t, 1, len(server.requests.createCERequests), "invalid number of requests") {
if tt.expectedRequest != nil && assert.Equal(
t, 1, len(server.requests.createCERequests), "invalid number of requests",
) {
r := server.requests.createCERequests[0]
assert.Equal(t, dummyAuthEmail, r.metadata.Get(authEmailKey)[0])
assert.Equal(t, dummyAuthToken, r.metadata.Get(authAPITokenKey)[0])
Expand Down Expand Up @@ -566,7 +567,7 @@ func TestClient_Cloud_UpdateExport(t *testing.T) {
data: &cloudexportpb.UpdateCloudExportResponse{Export: nil},
},
expectedResult: nil,
expectedError: true,
expectedError: true, // InvalidResponse
}, {
name: "AWS export updated",
request: newAWSExport(),
Expand All @@ -585,8 +586,12 @@ func TestClient_Cloud_UpdateExport(t *testing.T) {
},
expectedResult: newAWSExport(),
}, {
name: "update request validation, missing AWS.BUCKET",
request: newInvalidAWSExport(),
name: "AWS export with missing AWSProperties.Bucket field",
request: cloud.NewAWSExport(cloud.AWSExportRequiredFields{
Name: "invalid-aws-export",
PlanID: "11467",
AWSProperties: cloud.AWSPropertiesRequiredFields{},
}),
expectedResult: nil,
expectedError: true,
errorPredicates: []func(error) bool{kentikapi.IsInvalidRequestError},
Expand All @@ -613,7 +618,7 @@ func TestClient_Cloud_UpdateExport(t *testing.T) {
result, err := client.Cloud.UpdateExport(context.Background(), tt.request)

// assert
t.Logf("Got err: %v", err)
t.Logf("Got result: %+v, err: %v", result, err)
if tt.expectedError {
assert.Error(t, err)
for _, isErr := range tt.errorPredicates {
Expand All @@ -623,7 +628,9 @@ func TestClient_Cloud_UpdateExport(t *testing.T) {
assert.NoError(t, err)
}

if tt.expectedRequest != nil && assert.Equal(t, 1, len(server.requests.updateCERequests), "invalid number of requests") {
if tt.expectedRequest != nil && assert.Equal(
t, 1, len(server.requests.updateCERequests), "invalid number of requests",
) {
r := server.requests.updateCERequests[0]
assert.Equal(t, dummyAuthEmail, r.metadata.Get(authEmailKey)[0])
assert.Equal(t, dummyAuthToken, r.metadata.Get(authAPITokenKey)[0])
Expand All @@ -635,7 +642,6 @@ func TestClient_Cloud_UpdateExport(t *testing.T) {
}
}

//nolint:dupl
func TestClient_Cloud_DeleteExport(t *testing.T) {
tests := []struct {
name string
Expand Down Expand Up @@ -889,14 +895,6 @@ func newAWSExport() *cloud.Export {
return ce
}

func newInvalidAWSExport() *cloud.Export {
ce := newExport()
ce.ID = awsExportID
ce.Provider = cloud.ProviderAWS
ce.Properties = &cloud.AWSProperties{}
return ce
}

func newAzureExport() *cloud.Export {
ce := newExport()
ce.ID = azureExportID
Expand Down
Loading

0 comments on commit d23c437

Please sign in to comment.