-
Notifications
You must be signed in to change notification settings - Fork 129
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
feat: add support to validate via Kong API #502
Changes from 3 commits
231e0e7
b91c844
ef450c4
4136209
422f066
bc31676
e25214e
a4c603e
9c3ec69
94ed516
ddeb657
7523bfd
a6b0a11
34fb225
3e13393
6319328
536ba04
809e7d3
9c68a3b
69135ea
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 | ||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -1,29 +1,37 @@ | ||||||||||||||||||||||||||||||||||||||||||
package cmd | ||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||
import ( | ||||||||||||||||||||||||||||||||||||||||||
"context" | ||||||||||||||||||||||||||||||||||||||||||
"fmt" | ||||||||||||||||||||||||||||||||||||||||||
"reflect" | ||||||||||||||||||||||||||||||||||||||||||
"sync" | ||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||
"github.com/kong/deck/cprint" | ||||||||||||||||||||||||||||||||||||||||||
"github.com/kong/deck/file" | ||||||||||||||||||||||||||||||||||||||||||
"github.com/kong/deck/state" | ||||||||||||||||||||||||||||||||||||||||||
"github.com/kong/deck/utils" | ||||||||||||||||||||||||||||||||||||||||||
"github.com/kong/go-kong/kong" | ||||||||||||||||||||||||||||||||||||||||||
"github.com/spf13/cobra" | ||||||||||||||||||||||||||||||||||||||||||
) | ||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||
var ( | ||||||||||||||||||||||||||||||||||||||||||
validateCmdKongStateFile []string | ||||||||||||||||||||||||||||||||||||||||||
validateCmdRBACResourcesOnly bool | ||||||||||||||||||||||||||||||||||||||||||
validateOnline bool | ||||||||||||||||||||||||||||||||||||||||||
validateOnlineErrors []error | ||||||||||||||||||||||||||||||||||||||||||
) | ||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||
// validateCmd represents the diff command | ||||||||||||||||||||||||||||||||||||||||||
var validateCmd = &cobra.Command{ | ||||||||||||||||||||||||||||||||||||||||||
Use: "validate", | ||||||||||||||||||||||||||||||||||||||||||
Short: "Validate the state file", | ||||||||||||||||||||||||||||||||||||||||||
Long: `The validate command reads the state file and ensures validity. | ||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||
It reads all the specified state files and reports YAML/JSON | ||||||||||||||||||||||||||||||||||||||||||
parsing issues. It also checks for foreign relationships | ||||||||||||||||||||||||||||||||||||||||||
and alerts if there are broken relationships, or missing links present. | ||||||||||||||||||||||||||||||||||||||||||
No communication takes places between decK and Kong during the execution of | ||||||||||||||||||||||||||||||||||||||||||
this command. | ||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||
When ran with the --online flag, it also uses the 'validate' endpoint in Kong | ||||||||||||||||||||||||||||||||||||||||||
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.
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||
to validate entities configuration. | ||||||||||||||||||||||||||||||||||||||||||
`, | ||||||||||||||||||||||||||||||||||||||||||
Args: validateNoArgs, | ||||||||||||||||||||||||||||||||||||||||||
RunE: func(cmd *cobra.Command, args []string) error { | ||||||||||||||||||||||||||||||||||||||||||
|
@@ -50,11 +58,17 @@ this command. | |||||||||||||||||||||||||||||||||||||||||
return err | ||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||
// this catches foreign relation errors | ||||||||||||||||||||||||||||||||||||||||||
_, err = state.Get(rawState) | ||||||||||||||||||||||||||||||||||||||||||
ks, err := state.Get(rawState) | ||||||||||||||||||||||||||||||||||||||||||
if err != nil { | ||||||||||||||||||||||||||||||||||||||||||
return err | ||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||
if validateOnline { | ||||||||||||||||||||||||||||||||||||||||||
validateWithKong(cmd, ks) | ||||||||||||||||||||||||||||||||||||||||||
for _, e := range validateOnlineErrors { | ||||||||||||||||||||||||||||||||||||||||||
cprint.DeletePrintln(e) | ||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||
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. My bad, we don't print errors, we return the error from this command and cobra prints and exists appropriately. |
||||||||||||||||||||||||||||||||||||||||||
return nil | ||||||||||||||||||||||||||||||||||||||||||
}, | ||||||||||||||||||||||||||||||||||||||||||
PreRunE: func(cmd *cobra.Command, args []string) error { | ||||||||||||||||||||||||||||||||||||||||||
|
@@ -66,6 +80,57 @@ this command. | |||||||||||||||||||||||||||||||||||||||||
}, | ||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||
func validate(ctx context.Context, entity interface{}, kongClient *kong.Client, entityType string) error { | ||||||||||||||||||||||||||||||||||||||||||
_, err := validateEntity(ctx, kongClient, entityType, entity) | ||||||||||||||||||||||||||||||||||||||||||
if err != nil { | ||||||||||||||||||||||||||||||||||||||||||
return err | ||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||
return nil | ||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||
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. Get rid off this function and call validateEntity from validateEntities directly. |
||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||
func validateEntities(ctx context.Context, obj interface{}, kongClient *kong.Client, entityType string) { | ||||||||||||||||||||||||||||||||||||||||||
// call GetAll on entity | ||||||||||||||||||||||||||||||||||||||||||
method := reflect.ValueOf(obj).MethodByName("GetAll") | ||||||||||||||||||||||||||||||||||||||||||
entities := method.Call([]reflect.Value{})[0].Interface() | ||||||||||||||||||||||||||||||||||||||||||
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. Since there are no tests, we will need to figure out a way to catch an error here. 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. Not sure if I got you here, can you elaborate a bit please? We can also do a bit more of checks like this:
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. The goal here is to statically ensure that all these state structs have the GetAll method (cause otherwise the code will panic at runtime). One way to do that is to call all these reflect functions within an init function and then keep the references to the method in an array/map. Now, if there is a panic, it will happen on every call to main(). The likelihood of us catching that is much higher this way (until we have extensive testing happening for this project). 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 guess you mean something like this?
I haven't found a nice way to store the method reference and call it later at validation time though :\ (without having to create a proper state at init time) 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.
Why not do that? So Something like this: var store := map[string]somestruct{} func init(){ And then at runtime, use the store to call the methods? 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 initially I modelled exactly that, but this is going to implicitly add validation on the configuration file for all commands, right? I mean, if that's fine to you, that's fine to me :P Also, in order to render the state, I was reusing the code of the initial validate..this would mean that the current Or am I missing something? |
||||||||||||||||||||||||||||||||||||||||||
values := reflect.ValueOf(entities) | ||||||||||||||||||||||||||||||||||||||||||
var wg sync.WaitGroup | ||||||||||||||||||||||||||||||||||||||||||
wg.Add(values.Len()) | ||||||||||||||||||||||||||||||||||||||||||
for i := 0; i < values.Len(); i++ { | ||||||||||||||||||||||||||||||||||||||||||
go func(i int) { | ||||||||||||||||||||||||||||||||||||||||||
defer wg.Done() | ||||||||||||||||||||||||||||||||||||||||||
if err := validate(ctx, values.Index(i).Interface(), kongClient, entityType); err != nil { | ||||||||||||||||||||||||||||||||||||||||||
validateOnlineErrors = append(validateOnlineErrors, err) | ||||||||||||||||||||||||||||||||||||||||||
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. Please returns errors from this function instead of collecting these in a global variable. |
||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||
}(i) | ||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||
wg.Wait() | ||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||
func validateWithKong(cmd *cobra.Command, ks *state.KongState) error { | ||||||||||||||||||||||||||||||||||||||||||
ctx := cmd.Context() | ||||||||||||||||||||||||||||||||||||||||||
kongClient, err := utils.GetKongClient(rootConfig) | ||||||||||||||||||||||||||||||||||||||||||
if err != nil { | ||||||||||||||||||||||||||||||||||||||||||
return err | ||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||
validateEntities(ctx, ks.Services, kongClient, "services") | ||||||||||||||||||||||||||||||||||||||||||
validateEntities(ctx, ks.ACLGroups, kongClient, "acls") | ||||||||||||||||||||||||||||||||||||||||||
validateEntities(ctx, ks.BasicAuths, kongClient, "basicauth_credentials") | ||||||||||||||||||||||||||||||||||||||||||
validateEntities(ctx, ks.CACertificates, kongClient, "ca_certificates") | ||||||||||||||||||||||||||||||||||||||||||
validateEntities(ctx, ks.Certificates, kongClient, "certificates") | ||||||||||||||||||||||||||||||||||||||||||
validateEntities(ctx, ks.Consumers, kongClient, "consumers") | ||||||||||||||||||||||||||||||||||||||||||
validateEntities(ctx, ks.Documents, kongClient, "documents") | ||||||||||||||||||||||||||||||||||||||||||
validateEntities(ctx, ks.HMACAuths, kongClient, "hmacauth_credentials") | ||||||||||||||||||||||||||||||||||||||||||
validateEntities(ctx, ks.JWTAuths, kongClient, "jwt_secrets") | ||||||||||||||||||||||||||||||||||||||||||
validateEntities(ctx, ks.KeyAuths, kongClient, "keyauth_credentials") | ||||||||||||||||||||||||||||||||||||||||||
validateEntities(ctx, ks.Oauth2Creds, kongClient, "oauth2_credentials") | ||||||||||||||||||||||||||||||||||||||||||
validateEntities(ctx, ks.Plugins, kongClient, "plugins") | ||||||||||||||||||||||||||||||||||||||||||
validateEntities(ctx, ks.Routes, kongClient, "routes") | ||||||||||||||||||||||||||||||||||||||||||
validateEntities(ctx, ks.SNIs, kongClient, "snis") | ||||||||||||||||||||||||||||||||||||||||||
validateEntities(ctx, ks.Targets, kongClient, "targets") | ||||||||||||||||||||||||||||||||||||||||||
validateEntities(ctx, ks.Upstreams, kongClient, "upstreams") | ||||||||||||||||||||||||||||||||||||||||||
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. RBAC resources are missing here. Can you include those? 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. Ops, missed this...what is/are the db entities associated to RBAC resources? I couldn't find them :\ 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. Roles and endpoint permissions (there are some others, but we lack support for them). Lines 221 to 240 in 02208b2
https://docs.konghq.com/enterprise/2.5.x/admin-api/rbac/reference/#add-a-role 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. Thanks for the pointers! |
||||||||||||||||||||||||||||||||||||||||||
return nil | ||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||
func init() { | ||||||||||||||||||||||||||||||||||||||||||
rootCmd.AddCommand(validateCmd) | ||||||||||||||||||||||||||||||||||||||||||
validateCmd.Flags().BoolVar(&validateCmdRBACResourcesOnly, "rbac-resources-only", | ||||||||||||||||||||||||||||||||||||||||||
|
@@ -74,4 +139,6 @@ func init() { | |||||||||||||||||||||||||||||||||||||||||
"state", "s", []string{"kong.yaml"}, "file(s) containing Kong's configuration.\n"+ | ||||||||||||||||||||||||||||||||||||||||||
"This flag can be specified multiple times for multiple files.\n"+ | ||||||||||||||||||||||||||||||||||||||||||
"Use '-' to read from stdin.") | ||||||||||||||||||||||||||||||||||||||||||
validateCmd.Flags().BoolVar(&validateOnline, "online", | ||||||||||||||||||||||||||||||||||||||||||
false, "perform schema validation against Kong API.") | ||||||||||||||||||||||||||||||||||||||||||
GGabriele marked this conversation as resolved.
Show resolved
Hide resolved
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.
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please avoid a global state like this. Instead, change the function signatures to return
[]error
where needed.