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.

Sign up for GitHub

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

feat: add entities defaults from their schemas #573

Merged
merged 16 commits into from
Feb 11, 2022
Merged

feat: add entities defaults from their schemas #573

Show file tree
Hide file tree
Changes from 15 commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
adce1ab
feat: add entities defaults from their schemas
GGabriele Jan 27, 2022
f2ec428
Rework
GGabriele Jan 28, 2022
822b8d3
Fix test
GGabriele Jan 28, 2022
aae61fe
rework for precedence
GGabriele Jan 31, 2022
10c210d
Update go-kong
GGabriele Feb 1, 2022
2b35595
run update-codegen
GGabriele Feb 1, 2022
daa6921
update dependeces
GGabriele Feb 2, 2022
34e58a9
ignore Enabled field
GGabriele Feb 2, 2022
2688f83
get default constants back in
GGabriele Feb 7, 2022
4e445b4
use static defaults for konnect
GGabriele Feb 7, 2022
95d43c9
improve logic
GGabriele Feb 7, 2022
62615ed
handle case of strip_path and grpc protocols
GGabriele Feb 7, 2022
09a1e82
fix konnect nad add test
GGabriele Feb 8, 2022
e9d0bfd
introduce DefaulterOpts and unexport GetKongDefaulter
GGabriele Feb 9, 2022
35dd1c1
update dependeces
GGabriele Feb 9, 2022
67d1dcc
cleanup strip_path related code
GGabriele Feb 9, 2022
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
84 changes: 53 additions & 31 deletions file/builder.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@ type stateBuilder struct {

schemasCache map[string]map[string]interface{}

disableDynamicDefaults bool

err error
}

Expand All @@ -51,15 +53,11 @@ func (b *stateBuilder) build() (*utils.KongRawState, *utils.KonnectRawState, err
return nil, nil, err
}

// defaulter
var kongDefaults KongDefaults
if b.targetContent.Info != nil {
kongDefaults = b.targetContent.Info.Defaults
}
b.defaulter, err = defaulter(kongDefaults)
defaulter, err := defaulter(b.ctx, b.client, b.targetContent, b.disableDynamicDefaults)
if err != nil {
return nil, nil, fmt.Errorf("creating defaulter: %w", err)
return nil, nil, err
}
b.defaulter = defaulter

// build
b.certificates()
Expand Down Expand Up @@ -748,6 +746,32 @@ func (b *stateBuilder) plugins() {
}
}

// strip_path schema default value is 'true', but it cannot be set when
// protocols include 'grpc' and/or 'grpcs'. When users explicitly set
// strip_path to 'true' with grpc/s protocols, deck returns a schema violation error.
// When strip_path is not set and protocols include grpc/s, deck sets strip_path to 'false',
// despite its default value would be 'true' under normal circumstances.
func getStripPathBasedOnProtocols(initialStripPath *bool, route kong.Route) (*bool, error) {
var grpcProtocols bool
for _, tag := range route.Protocols {
if *tag == "grpc" || *tag == "grpcs" {
grpcProtocols = true
}
}

if grpcProtocols {
if initialStripPath != nil {
if *initialStripPath {
return nil, fmt.Errorf("schema violation (strip_path: cannot set " +
"'strip_path' when 'protocols' is 'grpc' or 'grpcs')")
}
return initialStripPath, nil
}
return kong.Bool(false), nil
}
Copy link
Member

Choose a reason for hiding this comment

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

Question: If strip path can't be set when protocol is grpc, why return false on the two returns above?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

strip_path is a required field, so when protocol is grpc it needs to be set to false (true being the schema default)

Copy link
Member

Choose a reason for hiding this comment

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

	for _, p := range route.Protocols {
		if *p == "grpc" || *p == "grpcs" {
			if stripPath != nil { return nil, fmt.Errorf("strip path not allowed")
      return kong.Bool(false), nil
		}
	}

Is that not sufficient?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Not directly...I just reworked/cleaned a bit the code to have something like you are suggesting (67d1dcc)

return route.StripPath, nil
}

func (b *stateBuilder) ingestRoute(r FRoute) error {
if utils.Empty(r.ID) {
route, err := b.currentState.Routes.Get(*r.Name)
Expand All @@ -761,10 +785,17 @@ func (b *stateBuilder) ingestRoute(r FRoute) error {
}

utils.MustMergeTags(&r, b.selectTags)

initialStripPath := r.Route.StripPath
b.defaulter.MustSet(&r.Route)
stripPath, err := getStripPathBasedOnProtocols(initialStripPath, r.Route)
if err != nil {
return err
}
r.Route.StripPath = stripPath

b.rawState.Routes = append(b.rawState.Routes, &r.Route)
err := b.intermediate.Routes.Add(state.Route{Route: r.Route})
err = b.intermediate.Routes.Add(state.Route{Route: r.Route})
if err != nil {
return err
}
Expand Down Expand Up @@ -868,30 +899,21 @@ func pluginRelations(plugin *kong.Plugin) (cID, rID, sID string) {
return
}

func defaulter(defaults KongDefaults) (*utils.Defaulter, error) {
d, err := utils.GetKongDefaulter()
if err != nil {
return nil, err
}
if defaults.Route != nil {
if err = d.Register(defaults.Route); err != nil {
return nil, err
}
}
if defaults.Service != nil {
if err = d.Register(defaults.Service); err != nil {
return nil, err
}
func defaulter(
ctx context.Context, client *kong.Client, fileContent *Content, disableDynamicDefaults bool,
) (*utils.Defaulter, error) {
var kongDefaults KongDefaults
if fileContent.Info != nil {
kongDefaults = fileContent.Info.Defaults
}
if defaults.Upstream != nil {
if err = d.Register(defaults.Upstream); err != nil {
return nil, err
}
opts := utils.DefaulterOpts{
Client: client,
KongDefaults: kongDefaults,
DisableDynamicDefaults: disableDynamicDefaults,
}
if defaults.Target != nil {
if err = d.Register(defaults.Target); err != nil {
return nil, err
}
defaulter, err := utils.GetDefaulter(ctx, opts)
if err != nil {
return nil, fmt.Errorf("creating defaulter: %w", err)
}
return d, nil
return defaulter, nil
}
Loading