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.

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

Merged
merged 20 commits into from
Jan 19, 2022
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
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
14 changes: 14 additions & 0 deletions cmd/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,20 @@ var (
assumeYes bool
)

func validateEntity(ctx context.Context, kongClient *kong.Client, entityType string, entity interface{}) (bool, error) {
errWrap := "validate entity '%s': %s"
endpoint := fmt.Sprintf("/schemas/%s/validate", entityType)
req, err := kongClient.NewRequest(http.MethodPost, endpoint, nil, entity)
if err != nil {
return false, fmt.Errorf(errWrap, entityType, err)
}
resp, err := kongClient.Do(ctx, req, nil)
if err != nil {
return false, fmt.Errorf(errWrap, entityType, err)
}
return resp.StatusCode == http.StatusOK, nil
}

// workspaceExists checks if workspace exists in Kong.
func workspaceExists(ctx context.Context, config utils.KongClientConfig, workspaceName string) (bool, error) {
rootConfig := config.ForWorkspace("")
Expand Down
75 changes: 71 additions & 4 deletions cmd/validate.go
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
Copy link
Member

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.

)

// 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
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
When ran with the --online flag, it also uses the 'validate' endpoint in Kong
No communication takes places between decK and Kong during the execution of
this command unless --online flag is used.

to validate entities configuration.
`,
Args: validateNoArgs,
RunE: func(cmd *cobra.Command, args []string) error {
Expand All @@ -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)
}
}
Copy link
Member

Choose a reason for hiding this comment

The 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.
You will need to implement an error type that can hold multiple errors and print those.
Similar to t

return nil
},
PreRunE: func(cmd *cobra.Command, args []string) error {
Expand All @@ -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
}
Copy link
Member

Choose a reason for hiding this comment

The 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()
Copy link
Member

Choose a reason for hiding this comment

The 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.
Can you use init() function to invoke code like this to ensure that it doesn't error/panic?

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 sure if I got you here, can you elaborate a bit please?

We can also do a bit more of checks like this:

method := reflect.ValueOf(obj).MethodByName("GetAll")
	if !method.IsValid() {
		// bla
	}
	entities := method.Call([]reflect.Value{})[0].Interface()
	values := reflect.ValueOf(entities)
	if values.Kind() != reflect.Slice {
		// bla
	}

Copy link
Member

Choose a reason for hiding this comment

The 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).

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I guess you mean something like this?

func callGetAll(obj interface{}) reflect.Value {
	// call GetAll method on entity
	method := reflect.ValueOf(obj).MethodByName("GetAll")
	entities := method.Call([]reflect.Value{})[0].Interface()
	return reflect.ValueOf(entities)
}

func validateGetAllMethod() {
	dummyEmptyState, _ := state.NewKongState()
	callGetAll(dummyEmptyState.Services)
	callGetAll(dummyEmptyState.ACLGroups)
	...
}

func init() {
	rootCmd.AddCommand(validateCmd)
	...
	validateGetAllMethod()
}

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)

Copy link
Member

Choose a reason for hiding this comment

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

(without having to create a proper state at init time)

Why not do that?

So Something like this:

var store := map[string]somestruct{}

func init(){
// get a reference to the right method, throw a panic if the method doesn't exist
method:=getMethod()
store[entityType]= somestruct{ method:method, other: releavant-info}
}

And then at runtime, use the store to call the methods?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The 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 validate behaviour will become implicit at runtime, and maybe the --online mode proposed here should become the "new default" for validate.

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)
Copy link
Member

Choose a reason for hiding this comment

The 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")
Copy link
Member

Choose a reason for hiding this comment

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

RBAC resources are missing here. Can you include those?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The 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 :\

Copy link
Contributor

Choose a reason for hiding this comment

The 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).

deck/dump/dump.go

Lines 221 to 240 in 02208b2

func getEnterpriseRBACConfiguration(ctx context.Context, group *errgroup.Group,
client *kong.Client, state *utils.KongRawState) {
group.Go(func() error {
roles, err := GetAllRBACRoles(ctx, client)
if err != nil {
return fmt.Errorf("roles: %w", err)
}
state.RBACRoles = roles
return nil
})
group.Go(func() error {
eps, err := GetAllRBACREndpointPermissions(ctx, client)
if err != nil {
return fmt.Errorf("eps: %w", err)
}
state.RBACEndpointPermissions = eps
return nil
})
}
is probably the best jumping-off point to find the associated functions and client structs.

https://docs.konghq.com/enterprise/2.5.x/admin-api/rbac/reference/#add-a-role
https://docs.konghq.com/enterprise/2.5.x/admin-api/rbac/reference/#add-a-role-endpoint-permission

Copy link
Collaborator Author

Choose a reason for hiding this comment

The 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",
Expand All @@ -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
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
false, "perform schema validation against Kong API.")
false, "perform validations against Kong API. When this flag is used, validation is done via communication with Kong. This increases the time for validation but catches significant errors. No resource is created in Kong.")

}
1 change: 1 addition & 0 deletions main.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ func registerSignalHandler() context.Context {
signal.Notify(sigs, syscall.SIGINT, syscall.SIGTERM)

go func() {
defer signal.Stop(sigs)
GGabriele marked this conversation as resolved.
Show resolved Hide resolved
sig := <-sigs
fmt.Println("received", sig, ", terminating...")
cancel()
Expand Down