Skip to content

Commit

Permalink
Fix: Agent killed when API applied with no mocking or upstream defined (
Browse files Browse the repository at this point in the history
#411)

* Fix issue where manager crashes if neither mocking is set nor upstream set globally or in a way that covers all paths and methods.

* correct routing docs
  • Loading branch information
Kyle Hodgetts authored May 14, 2022
1 parent d08f883 commit 6434040
Show file tree
Hide file tree
Showing 6 changed files with 103 additions and 24 deletions.
2 changes: 1 addition & 1 deletion api/v1alpha1/api_webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ func (a *APIValidator) Handle(ctx context.Context, req admission.Request) admiss
return admission.Errored(http.StatusBadRequest, err)
}
if err := apiObj.validate(); err != nil {
return admission.Errored(http.StatusInternalServerError, err)
return admission.Errored(http.StatusBadRequest, err)
}

return admission.Allowed("")
Expand Down
44 changes: 26 additions & 18 deletions docs/guides/routing.md
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,9 @@ info:
version: 0.1.0
x-kusk:
upstream:
service: simple-api-service
namespace: default
service:
name: simple-api-service
namespace: default
..
```

Expand All @@ -42,17 +43,19 @@ info:
version: 0.1.0
x-kusk:
upstream:
service: simple-api-service
namespace: default
port: 8001
service:
name: simple-api-service
namespace: default
port: 8001
path:
/someoperation:
get:
operationId: doSomething
x-kusk:
upstream:
service: another-service
port: 8080
service:
name: another-service
port: 8080
..
```

Expand All @@ -69,8 +72,9 @@ info:
version: 0.1.0
x-kusk:
upstream:
host: simple-api-service-hostname
port: 8001
host:
hostname: simple-api-service-hostname
port: 8001
..
```

Expand All @@ -83,16 +87,18 @@ info:
version: 0.1.0
x-kusk:
upstream:
host: simple-api-service-hostname
port: 8001
host:
hostname: simple-api-service-hostname
port: 8001
path:
/someoperation:
get:
operationId: doSomething
x-kusk:
upstream:
service: another-service
port: 8080
service:
name: another-service
port: 8080
..
```

Expand Down Expand Up @@ -182,8 +188,9 @@ info:
version: 0.1.0
x-kusk:
upstream:
host: simple-api-service-hostname
port: 8001
host:
hostname: simple-api-service-hostname
port: 8001
path:
/someoperation:
get:
Expand Down Expand Up @@ -260,8 +267,9 @@ info:
version: 0.1.0
x-kusk:
upstream:
host: simple-api-service-hostname
port: 8001
host:
hostname: simple-api-service-hostname
port: 8001
path:
/someoperation:
get:
Expand All @@ -273,4 +281,4 @@ path:
```

Setting this property to true at the top level will hide all operations, allowing you to then override this
property at the path or operation level if you want to expose only specific operations.
property at the path or operation level if you want to expose only specific operations.
23 changes: 23 additions & 0 deletions internal/agent/management/management.pb.go

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

2 changes: 1 addition & 1 deletion pkg/options/mocking.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ SOFTWARE.
package options

type MockingOptions struct {
Enabled *bool `json:"enabled" yaml:"mocking"`
Enabled *bool `json:"enabled" yaml:"enabled"`
}

func (o MockingOptions) Validate() error {
Expand Down
39 changes: 35 additions & 4 deletions pkg/options/options.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,13 +49,46 @@ func (o *Options) FillDefaults() {
if len(o.Hosts) == 0 {
o.Hosts = append(o.Hosts, "*")
}

if o.Upstream != nil {
o.Upstream.FillDefaults()
}
}

func (o Options) Validate() error {
return v.ValidateStruct(&o,
if err := v.ValidateStruct(&o,
v.Field(&o.Hosts, v.Each()),
v.Field(&o.OperationFinalSubOptions, v.Each()),
)
); err != nil {
return err
}

// check if global options contains either mocking or an upstream service that covers all endpoints
if o.Mocking != nil {
return o.Mocking.Validate()
}

if o.Upstream != nil {
return o.Upstream.Validate()
}

// if we get here, it means there aren't global options contains either mocking or an upstream service that covers all endpoints
// therefore we need to iterate over all operations and check if they have either mocking or an upstream service
for pathAndMethod, op := range o.OperationFinalSubOptions {
if op.Mocking != nil {
return o.Mocking.Validate()
}

if op.Upstream != nil {
return o.Upstream.Validate()
}

// if we reach here then this path that doesn't have either mocking or an upstream service and is not covered by a
// global upstream service or mocking config, so return an error
return fmt.Errorf("no upstream or mocking configuration found for path %s", pathAndMethod)
}

return nil
}

// SubOptions allow user to overwrite certain options at path/operation level using x-kusk extension
Expand Down Expand Up @@ -162,6 +195,4 @@ func (o *SubOptions) MergeInSubOptions(in *SubOptions) {
if o.RateLimit == nil && in.RateLimit != nil {
o.RateLimit = in.RateLimit
}

return
}
17 changes: 17 additions & 0 deletions pkg/options/upstream.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,12 @@ type UpstreamOptions struct {
Rewrite RewriteRegex `yaml:"rewrite,omitempty" json:"rewrite,omitempty"`
}

func (o *UpstreamOptions) FillDefaults() {
if o.Service != nil {
o.Service.FillDefaults()
}
}

// UpstreamHost defines any DNS hostname with port that we can proxy to, even outside of the cluster
type UpstreamHost struct {
// Hostname is the upstream hostname, without port.
Expand Down Expand Up @@ -72,6 +78,17 @@ func (o UpstreamHost) Validate() error {
v.Field(&o.Port, v.Min(uint32(1)), v.Max(uint32(65356)), v.Required),
)
}

func (o *UpstreamService) FillDefaults() {
if o.Namespace == "" {
o.Namespace = "default"
}

if o.Port == 0 {
o.Port = 80
}
}

func (o UpstreamService) Validate() error {
return v.ValidateStruct(&o,
v.Field(&o.Name, is.DNSName, v.Required),
Expand Down

0 comments on commit 6434040

Please sign in to comment.