From e8bc38f25b22962de35372a6fac27c8827b873d8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Charles-Edouard=20Br=C3=A9t=C3=A9ch=C3=A9?= Date: Tue, 5 Sep 2023 19:10:27 +0200 Subject: [PATCH] refactor: introduce userinfo package in the cli (#8272) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Charles-Edouard Brétéché --- .../kubectl-kyverno/commands/apply/command.go | 7 ++- cmd/cli/kubectl-kyverno/commands/test/test.go | 5 +- .../testdata/user-infos/invalid.yaml | 1 + .../testdata/user-infos/valid.yaml | 4 ++ cmd/cli/kubectl-kyverno/userinfo/userinfo.go | 39 ++++++++++++ .../kubectl-kyverno/userinfo/userinfo_test.go | 60 +++++++++++++++++++ .../kubectl-kyverno/utils/common/common.go | 47 +-------------- .../utils/common/common_test.go | 3 +- .../utils/common/kyverno_policies_types.go | 2 +- .../limit-configmap-for-sa/kyverno-test.yaml | 1 - .../limit-configmap-for-sa/user_info.yaml | 4 -- 11 files changed, 114 insertions(+), 59 deletions(-) create mode 100644 cmd/cli/kubectl-kyverno/testdata/user-infos/invalid.yaml create mode 100644 cmd/cli/kubectl-kyverno/testdata/user-infos/valid.yaml create mode 100644 cmd/cli/kubectl-kyverno/userinfo/userinfo.go create mode 100644 cmd/cli/kubectl-kyverno/userinfo/userinfo_test.go delete mode 100644 test/cli/test/limit-configmap-for-sa/user_info.yaml diff --git a/cmd/cli/kubectl-kyverno/commands/apply/command.go b/cmd/cli/kubectl-kyverno/commands/apply/command.go index 031368fc57..8ae35e8068 100644 --- a/cmd/cli/kubectl-kyverno/commands/apply/command.go +++ b/cmd/cli/kubectl-kyverno/commands/apply/command.go @@ -14,6 +14,7 @@ import ( "github.com/kyverno/kyverno/api/kyverno/v1beta1" "github.com/kyverno/kyverno/cmd/cli/kubectl-kyverno/output/color" "github.com/kyverno/kyverno/cmd/cli/kubectl-kyverno/test/api" + "github.com/kyverno/kyverno/cmd/cli/kubectl-kyverno/userinfo" cobrautils "github.com/kyverno/kyverno/cmd/cli/kubectl-kyverno/utils/cobra" "github.com/kyverno/kyverno/cmd/cli/kubectl-kyverno/utils/common" reportutils "github.com/kyverno/kyverno/cmd/cli/kubectl-kyverno/utils/report" @@ -138,9 +139,9 @@ func (c *ApplyCommandConfig) applyCommandHelper() (*common.ResultCounts, []*unst if err != nil { return rc, uu, skipInvalidPolicies, er, err } - var userInfo v1beta1.RequestInfo + var userInfo *v1beta1.RequestInfo if c.UserInfoPath != "" { - userInfo, err = common.GetUserInfoFromPath(nil, c.UserInfoPath, "") + userInfo, err = userinfo.Load(nil, c.UserInfoPath, "") if err != nil { fmt.Printf("Error: failed to load request info\nCause: %s\n", err) osExit(1) @@ -211,7 +212,7 @@ func (c *ApplyCommandConfig) applyValidatingAdmissionPolicytoResource(validating return rc, resources, skipInvalidPolicies, responses, nil } -func (c *ApplyCommandConfig) applyPolicytoResource(variables map[string]string, policies []kyvernov1.PolicyInterface, validatingAdmissionPolicies []v1alpha1.ValidatingAdmissionPolicy, resources []*unstructured.Unstructured, openApiManager openapi.Manager, skipInvalidPolicies SkippedInvalidPolicies, valuesMap map[string]map[string]api.Resource, dClient dclient.Interface, subresources []api.Subresource, globalValMap map[string]string, userInfo v1beta1.RequestInfo, mutateLogPathIsDir bool, namespaceSelectorMap map[string]map[string]string) (*common.ResultCounts, []*unstructured.Unstructured, SkippedInvalidPolicies, []engineapi.EngineResponse, error) { +func (c *ApplyCommandConfig) applyPolicytoResource(variables map[string]string, policies []kyvernov1.PolicyInterface, validatingAdmissionPolicies []v1alpha1.ValidatingAdmissionPolicy, resources []*unstructured.Unstructured, openApiManager openapi.Manager, skipInvalidPolicies SkippedInvalidPolicies, valuesMap map[string]map[string]api.Resource, dClient dclient.Interface, subresources []api.Subresource, globalValMap map[string]string, userInfo *v1beta1.RequestInfo, mutateLogPathIsDir bool, namespaceSelectorMap map[string]map[string]string) (*common.ResultCounts, []*unstructured.Unstructured, SkippedInvalidPolicies, []engineapi.EngineResponse, error) { if len(variables) != 0 { variables = common.SetInStoreContext(policies, variables) } diff --git a/cmd/cli/kubectl-kyverno/commands/test/test.go b/cmd/cli/kubectl-kyverno/commands/test/test.go index 703d17dbb0..a07b5dfd13 100644 --- a/cmd/cli/kubectl-kyverno/commands/test/test.go +++ b/cmd/cli/kubectl-kyverno/commands/test/test.go @@ -7,6 +7,7 @@ import ( "github.com/kyverno/kyverno/cmd/cli/kubectl-kyverno/output/pluralize" "github.com/kyverno/kyverno/cmd/cli/kubectl-kyverno/resource" "github.com/kyverno/kyverno/cmd/cli/kubectl-kyverno/test" + "github.com/kyverno/kyverno/cmd/cli/kubectl-kyverno/userinfo" "github.com/kyverno/kyverno/cmd/cli/kubectl-kyverno/utils/common" pathutils "github.com/kyverno/kyverno/cmd/cli/kubectl-kyverno/utils/path" sanitizederror "github.com/kyverno/kyverno/cmd/cli/kubectl-kyverno/utils/sanitizedError" @@ -48,10 +49,10 @@ func runTest(openApiManager openapi.Manager, testCase test.TestCase, auditWarn b return nil, err } // user info - var userInfo v1beta1.RequestInfo + var userInfo *v1beta1.RequestInfo if testCase.Test.UserInfo != "" { fmt.Println(" Loading user infos", "...") - userInfo, err = common.GetUserInfoFromPath(testCase.Fs, testCase.Test.UserInfo, testDir) + userInfo, err = userinfo.Load(testCase.Fs, testCase.Test.UserInfo, testDir) if err != nil { return nil, fmt.Errorf("Error: failed to load request info (%s)", err) } diff --git a/cmd/cli/kubectl-kyverno/testdata/user-infos/invalid.yaml b/cmd/cli/kubectl-kyverno/testdata/user-infos/invalid.yaml new file mode 100644 index 0000000000..7daacd5db8 --- /dev/null +++ b/cmd/cli/kubectl-kyverno/testdata/user-infos/invalid.yaml @@ -0,0 +1 @@ +foo: bar \ No newline at end of file diff --git a/cmd/cli/kubectl-kyverno/testdata/user-infos/valid.yaml b/cmd/cli/kubectl-kyverno/testdata/user-infos/valid.yaml new file mode 100644 index 0000000000..0fd5541e98 --- /dev/null +++ b/cmd/cli/kubectl-kyverno/testdata/user-infos/valid.yaml @@ -0,0 +1,4 @@ +clusterRoles: +- cluster-admin +userInfo: + username: molybdenum@somecorp.com \ No newline at end of file diff --git a/cmd/cli/kubectl-kyverno/userinfo/userinfo.go b/cmd/cli/kubectl-kyverno/userinfo/userinfo.go new file mode 100644 index 0000000000..0773d46bf2 --- /dev/null +++ b/cmd/cli/kubectl-kyverno/userinfo/userinfo.go @@ -0,0 +1,39 @@ +package userinfo + +import ( + "fmt" + "io" + "os" + "path/filepath" + + "github.com/go-git/go-billy/v5" + kyvernov1beta1 "github.com/kyverno/kyverno/api/kyverno/v1beta1" + sanitizederror "github.com/kyverno/kyverno/cmd/cli/kubectl-kyverno/utils/sanitizedError" + "sigs.k8s.io/yaml" +) + +func Load(fs billy.Filesystem, path string, resourcePath string) (*kyvernov1beta1.RequestInfo, error) { + var userInfo kyvernov1beta1.RequestInfo + if fs != nil { + filep, err := fs.Open(filepath.Join(resourcePath, path)) + if err != nil { + return nil, fmt.Errorf("Unable to open userInfo file: %s. \nerror: %s", path, err) + } + bytes, err := io.ReadAll(filep) + if err != nil { + return nil, fmt.Errorf("Error: failed to read file %s: %w", filep.Name(), err) + } + if err := yaml.UnmarshalStrict(bytes, &userInfo); err != nil { + return nil, sanitizederror.NewWithError("failed to decode yaml", err) + } + } else { + bytes, err := os.ReadFile(filepath.Clean(filepath.Join(resourcePath, path))) + if err != nil { + return nil, sanitizederror.NewWithError("unable to read yaml", err) + } + if err := yaml.UnmarshalStrict(bytes, &userInfo); err != nil { + return nil, sanitizederror.NewWithError("failed to decode yaml", err) + } + } + return &userInfo, nil +} diff --git a/cmd/cli/kubectl-kyverno/userinfo/userinfo_test.go b/cmd/cli/kubectl-kyverno/userinfo/userinfo_test.go new file mode 100644 index 0000000000..3c4c315ccc --- /dev/null +++ b/cmd/cli/kubectl-kyverno/userinfo/userinfo_test.go @@ -0,0 +1,60 @@ +package userinfo + +import ( + "reflect" + "testing" + + "github.com/go-git/go-billy/v5" + kyvernov1beta1 "github.com/kyverno/kyverno/api/kyverno/v1beta1" + authenticationv1 "k8s.io/api/authentication/v1" +) + +func TestLoad(t *testing.T) { + tests := []struct { + name string + fs billy.Filesystem + path string + resourcePath string + want *kyvernov1beta1.RequestInfo + wantErr bool + }{{ + name: "empty", + fs: nil, + path: "", + resourcePath: "", + want: nil, + wantErr: true, + }, { + name: "invalid", + fs: nil, + path: "../testdata/user-infos/invalid.yaml", + resourcePath: "", + want: nil, + wantErr: true, + }, { + name: "valid", + fs: nil, + path: "../testdata/user-infos/valid.yaml", + resourcePath: "", + want: &kyvernov1beta1.RequestInfo{ + ClusterRoles: []string{"cluster-admin"}, + AdmissionUserInfo: authenticationv1.UserInfo{ + Username: "molybdenum@somecorp.com", + }, + }, + wantErr: false, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := Load(tt.fs, tt.path, tt.resourcePath) + if (err != nil) != tt.wantErr { + t.Errorf("Load() error = %v, wantErr %v", err, tt.wantErr) + return + } + if !reflect.DeepEqual(got, tt.want) { + t.Errorf("Load() = %v, want %v", got, tt.want) + } + }) + } +} diff --git a/cmd/cli/kubectl-kyverno/utils/common/common.go b/cmd/cli/kubectl-kyverno/utils/common/common.go index c90748f75e..4627de25e3 100644 --- a/cmd/cli/kubectl-kyverno/utils/common/common.go +++ b/cmd/cli/kubectl-kyverno/utils/common/common.go @@ -3,7 +3,6 @@ package common import ( "bufio" "context" - "encoding/json" "fmt" "io" "net/http" @@ -58,7 +57,7 @@ type ApplyPolicyConfig struct { MutateLogPath string MutateLogPathIsDir bool Variables map[string]interface{} - UserInfo kyvernov1beta1.RequestInfo + UserInfo *kyvernov1beta1.RequestInfo PolicyReport bool NamespaceSelectorMap map[string]map[string]string Stdin bool @@ -583,50 +582,6 @@ func handleGeneratePolicy(generateResponse *engineapi.EngineResponse, policyCont return newRuleResponse, nil } -// GetUserInfoFromPath - get the request info as user info from a given path -func GetUserInfoFromPath(fs billy.Filesystem, path string, policyResourcePath string) (kyvernov1beta1.RequestInfo, error) { - userInfo := &kyvernov1beta1.RequestInfo{} - if fs != nil { - filep, err := fs.Open(filepath.Join(policyResourcePath, path)) - if err != nil { - fmt.Printf("Unable to open userInfo file: %s. \nerror: %s", path, err) - } - bytes, err := io.ReadAll(filep) - if err != nil { - fmt.Printf("Error: failed to read file %s: %v", filep.Name(), err.Error()) - } - userInfoBytes, err := yaml.ToJSON(bytes) - if err != nil { - fmt.Printf("failed to convert to JSON: %v", err) - } - - if err := json.Unmarshal(userInfoBytes, userInfo); err != nil { - fmt.Printf("failed to decode yaml: %v", err) - } - } else { - var errors []error - pathname := filepath.Clean(filepath.Join(policyResourcePath, path)) - bytes, err := os.ReadFile(pathname) - if err != nil { - errors = append(errors, sanitizederror.NewWithError("unable to read yaml", err)) - } - userInfoBytes, err := yaml.ToJSON(bytes) - if err != nil { - errors = append(errors, sanitizederror.NewWithError("failed to convert json", err)) - } - if err := json.Unmarshal(userInfoBytes, userInfo); err != nil { - errors = append(errors, sanitizederror.NewWithError("failed to decode yaml", err)) - } - if len(errors) > 0 && log.V(1).Enabled() { - fmt.Printf("ignoring errors: \n") - for _, e := range errors { - fmt.Printf(" %v \n", e.Error()) - } - } - } - return *userInfo, nil -} - func GetGitBranchOrPolicyPaths(gitBranch, repoURL string, policyPaths []string) (string, string) { var gitPathToYamls string if gitBranch == "" { diff --git a/cmd/cli/kubectl-kyverno/utils/common/common_test.go b/cmd/cli/kubectl-kyverno/utils/common/common_test.go index 0641b18cc6..d204c136f7 100644 --- a/cmd/cli/kubectl-kyverno/utils/common/common_test.go +++ b/cmd/cli/kubectl-kyverno/utils/common/common_test.go @@ -3,7 +3,6 @@ package common import ( "testing" - "github.com/kyverno/kyverno/api/kyverno/v1beta1" "github.com/kyverno/kyverno/cmd/cli/kubectl-kyverno/resource" "github.com/kyverno/kyverno/cmd/cli/kubectl-kyverno/test/api" yamlutils "github.com/kyverno/kyverno/pkg/utils/yaml" @@ -105,7 +104,7 @@ func Test_NamespaceSelector(t *testing.T) { Policy: policyArray[0], Resource: resourceArray[0], MutateLogPath: "", - UserInfo: v1beta1.RequestInfo{}, + UserInfo: nil, NamespaceSelectorMap: tc.namespaceSelectorMap, Rc: rc, } diff --git a/cmd/cli/kubectl-kyverno/utils/common/kyverno_policies_types.go b/cmd/cli/kubectl-kyverno/utils/common/kyverno_policies_types.go index 36d8167490..d0327c2e28 100644 --- a/cmd/cli/kubectl-kyverno/utils/common/kyverno_policies_types.go +++ b/cmd/cli/kubectl-kyverno/utils/common/kyverno_policies_types.go @@ -133,7 +133,7 @@ OuterLoop: jp, *updatedResource, operation, - &c.UserInfo, + c.UserInfo, cfg, ) if err != nil { diff --git a/test/cli/test/limit-configmap-for-sa/kyverno-test.yaml b/test/cli/test/limit-configmap-for-sa/kyverno-test.yaml index 566881fb9c..91545a2ceb 100644 --- a/test/cli/test/limit-configmap-for-sa/kyverno-test.yaml +++ b/test/cli/test/limit-configmap-for-sa/kyverno-test.yaml @@ -17,5 +17,4 @@ results: - any-configmap-name-bad result: skip rule: limit-configmap-for-sa-developer -userinfo: user_info.yaml variables: variables.yaml diff --git a/test/cli/test/limit-configmap-for-sa/user_info.yaml b/test/cli/test/limit-configmap-for-sa/user_info.yaml deleted file mode 100644 index 7e96ea966c..0000000000 --- a/test/cli/test/limit-configmap-for-sa/user_info.yaml +++ /dev/null @@ -1,4 +0,0 @@ -subject: - kind: ServiceAccount - name: another-developer - namespace: another-namespace \ No newline at end of file