From 4c6d5579c50e504da66db535d2fbea730c1eb5c9 Mon Sep 17 00:00:00 2001 From: Michael Crenshaw Date: Fri, 8 Apr 2022 11:23:36 -0400 Subject: [PATCH 1/5] chore: eliminate go-mpatch dependency Signed-off-by: Michael Crenshaw --- .../commands/admin/project_allowlist.go | 17 +++--- .../commands/admin/project_allowlist_test.go | 54 ++----------------- go.mod | 1 - go.sum | 2 - 4 files changed, 13 insertions(+), 61 deletions(-) diff --git a/cmd/argocd/commands/admin/project_allowlist.go b/cmd/argocd/commands/admin/project_allowlist.go index 2d9dd85b363fe..aeac15a08585f 100644 --- a/cmd/argocd/commands/admin/project_allowlist.go +++ b/cmd/argocd/commands/admin/project_allowlist.go @@ -63,7 +63,13 @@ func NewProjectAllowListGenCommand() *cobra.Command { }() } - globalProj := generateProjectAllowList(clientConfig, clusterRoleFileName, projName) + config, err := clientConfig.ClientConfig() + errors.CheckError(err) + disco, err := discovery.NewDiscoveryClientForConfig(config) + errors.CheckError(err) + serverResources, err := disco.ServerPreferredResources() + errors.CheckError(err) + globalProj := generateProjectAllowList(serverResources, clusterRoleFileName, projName) yamlBytes, err := yaml.Marshal(globalProj) errors.CheckError(err) @@ -78,7 +84,7 @@ func NewProjectAllowListGenCommand() *cobra.Command { return command } -func generateProjectAllowList(clientConfig clientcmd.ClientConfig, clusterRoleFileName string, projName string) v1alpha1.AppProject { +func generateProjectAllowList(serverResources []*metav1.APIResourceList, clusterRoleFileName string, projName string) v1alpha1.AppProject { yamlBytes, err := ioutil.ReadFile(clusterRoleFileName) errors.CheckError(err) var obj unstructured.Unstructured @@ -89,13 +95,6 @@ func generateProjectAllowList(clientConfig clientcmd.ClientConfig, clusterRoleFi err = scheme.Scheme.Convert(&obj, clusterRole, nil) errors.CheckError(err) - config, err := clientConfig.ClientConfig() - errors.CheckError(err) - disco, err := discovery.NewDiscoveryClientForConfig(config) - errors.CheckError(err) - serverResources, err := disco.ServerPreferredResources() - errors.CheckError(err) - resourceList := make([]metav1.GroupKind, 0) for _, rule := range clusterRole.Rules { if len(rule.APIGroups) <= 0 { diff --git a/cmd/argocd/commands/admin/project_allowlist_test.go b/cmd/argocd/commands/admin/project_allowlist_test.go index 0d57db932fda4..bb370e742bd44 100644 --- a/cmd/argocd/commands/admin/project_allowlist_test.go +++ b/cmd/argocd/commands/admin/project_allowlist_test.go @@ -1,63 +1,19 @@ package admin import ( - "reflect" - "runtime" "testing" "github.com/stretchr/testify/assert" - "github.com/undefinedlabs/go-mpatch" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/client-go/discovery" - restclient "k8s.io/client-go/rest" - "k8s.io/client-go/tools/clientcmd" ) func TestProjectAllowListGen(t *testing.T) { - // go-mpatch only works on platforms with amd64 architecture - if runtime.GOARCH != "amd64" { - t.Skip() + res := metav1.APIResource{ + Name: "services", + Kind: "Service", } + resourceList := []*metav1.APIResourceList{{APIResources: []metav1.APIResource{res}}} - useMock := true - rules := clientcmd.NewDefaultClientConfigLoadingRules() - overrides := &clientcmd.ConfigOverrides{} - clientConfig := clientcmd.NewNonInteractiveDeferredLoadingClientConfig(rules, overrides) - - if useMock { - var patchClientConfig *mpatch.Patch - patchClientConfig, err := mpatch.PatchInstanceMethodByName(reflect.TypeOf(clientConfig), "ClientConfig", func(*clientcmd.DeferredLoadingClientConfig) (*restclient.Config, error) { - return nil, nil - }) - assert.NoError(t, err) - - patch, err := mpatch.PatchMethod(discovery.NewDiscoveryClientForConfig, func(c *restclient.Config) (*discovery.DiscoveryClient, error) { - return &discovery.DiscoveryClient{LegacyPrefix: "/api"}, nil - }) - assert.NoError(t, err) - - var patchSeverPreferredResources *mpatch.Patch - discoClient := &discovery.DiscoveryClient{} - patchSeverPreferredResources, err = mpatch.PatchInstanceMethodByName(reflect.TypeOf(discoClient), "ServerPreferredResources", func(*discovery.DiscoveryClient) ([]*metav1.APIResourceList, error) { - res := metav1.APIResource{ - Name: "services", - Kind: "Service", - } - resourceList := []*metav1.APIResourceList{{APIResources: []metav1.APIResource{res}}} - return resourceList, nil - }) - assert.NoError(t, err) - - defer func() { - err = patchClientConfig.Unpatch() - assert.NoError(t, err) - err = patch.Unpatch() - assert.NoError(t, err) - err = patchSeverPreferredResources.Unpatch() - err = patch.Unpatch() - }() - } - - globalProj := generateProjectAllowList(clientConfig, "testdata/test_clusterrole.yaml", "testproj") + globalProj := generateProjectAllowList(resourceList, "testdata/test_clusterrole.yaml", "testproj") assert.True(t, len(globalProj.Spec.NamespaceResourceWhitelist) > 0) } diff --git a/go.mod b/go.mod index f1121a85bde4d..bcaf2d714c46a 100644 --- a/go.mod +++ b/go.mod @@ -69,7 +69,6 @@ require ( github.com/spf13/cobra v1.3.0 github.com/spf13/pflag v1.0.5 github.com/stretchr/testify v1.7.0 - github.com/undefinedlabs/go-mpatch v1.0.6 github.com/valyala/fasttemplate v1.2.1 github.com/whilp/git-urls v0.0.0-20191001220047-6db9661140c0 github.com/xanzy/go-gitlab v0.60.0 diff --git a/go.sum b/go.sum index e61c20c0d4fa6..2bb6fb9f06465 100644 --- a/go.sum +++ b/go.sum @@ -1064,8 +1064,6 @@ github.com/tv42/httpunix v0.0.0-20150427012821-b75d8614f926/go.mod h1:9ESjWnEqri github.com/ugorji/go v1.1.4/go.mod h1:uQMGLiO92mf5W77hV/PUCpI3pbzQx3CRekS0kk+RGrc= github.com/ugorji/go v1.1.7/go.mod h1:kZn38zHttfInRq0xu/PH0az30d+z6vm202qpg1oXVMw= github.com/ugorji/go/codec v1.1.7/go.mod h1:Ax+UKWsSmolVDwsd+7N3ZtXu+yMGCf907BLYF3GoBXY= -github.com/undefinedlabs/go-mpatch v1.0.6 h1:h8q5ORH/GaOE1Se1DMhrOyljXZEhRcROO7agMqWXCOY= -github.com/undefinedlabs/go-mpatch v1.0.6/go.mod h1:TyJZDQ/5AgyN7FSLiBJ8RO9u2c6wbtRvK827b6AVqY4= github.com/urfave/cli v1.20.0/go.mod h1:70zkFmudgCuE/ngEzBv17Jvp/497gISqfk5gWijbERA= github.com/urfave/cli v1.22.1/go.mod h1:Gos4lmkARVdJ6EkW0WaNv/tZAAMe9V7XWyB60NtXRu0= github.com/urfave/cli v1.22.2/go.mod h1:Gos4lmkARVdJ6EkW0WaNv/tZAAMe9V7XWyB60NtXRu0= From 3d0590cd26d39f1a597b8bf6a52992503c4ccbb6 Mon Sep 17 00:00:00 2001 From: Michael Crenshaw Date: Fri, 8 Apr 2022 17:33:39 -0400 Subject: [PATCH 2/5] chore: abstract out resource list function Signed-off-by: Michael Crenshaw --- cmd/argocd/commands/admin/project_allowlist.go | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/cmd/argocd/commands/admin/project_allowlist.go b/cmd/argocd/commands/admin/project_allowlist.go index aeac15a08585f..3532a3b460338 100644 --- a/cmd/argocd/commands/admin/project_allowlist.go +++ b/cmd/argocd/commands/admin/project_allowlist.go @@ -63,13 +63,7 @@ func NewProjectAllowListGenCommand() *cobra.Command { }() } - config, err := clientConfig.ClientConfig() - errors.CheckError(err) - disco, err := discovery.NewDiscoveryClientForConfig(config) - errors.CheckError(err) - serverResources, err := disco.ServerPreferredResources() - errors.CheckError(err) - globalProj := generateProjectAllowList(serverResources, clusterRoleFileName, projName) + globalProj := generateProjectAllowList(getResourceList(clientConfig), clusterRoleFileName, projName) yamlBytes, err := yaml.Marshal(globalProj) errors.CheckError(err) @@ -84,6 +78,16 @@ func NewProjectAllowListGenCommand() *cobra.Command { return command } +func getResourceList(clientConfig clientcmd.ClientConfig) []*metav1.APIResourceList { + config, err := clientConfig.ClientConfig() + errors.CheckError(err) + disco, err := discovery.NewDiscoveryClientForConfig(config) + errors.CheckError(err) + serverResources, err := disco.ServerPreferredResources() + errors.CheckError(err) + return serverResources +} + func generateProjectAllowList(serverResources []*metav1.APIResourceList, clusterRoleFileName string, projName string) v1alpha1.AppProject { yamlBytes, err := ioutil.ReadFile(clusterRoleFileName) errors.CheckError(err) From 5dec981355c070da07692920c3285615b4bf7e3c Mon Sep 17 00:00:00 2001 From: Michael Crenshaw Date: Fri, 8 Apr 2022 17:50:30 -0400 Subject: [PATCH 3/5] chore: don't exit the program in anything but the main function Signed-off-by: Michael Crenshaw --- .../commands/admin/project_allowlist.go | 37 +++++++++++++------ .../commands/admin/project_allowlist_test.go | 3 +- 2 files changed, 28 insertions(+), 12 deletions(-) diff --git a/cmd/argocd/commands/admin/project_allowlist.go b/cmd/argocd/commands/admin/project_allowlist.go index 3532a3b460338..835ae02cd2ff1 100644 --- a/cmd/argocd/commands/admin/project_allowlist.go +++ b/cmd/argocd/commands/admin/project_allowlist.go @@ -63,7 +63,10 @@ func NewProjectAllowListGenCommand() *cobra.Command { }() } - globalProj := generateProjectAllowList(getResourceList(clientConfig), clusterRoleFileName, projName) + resourceList, err := getResourceList(clientConfig) + errors.CheckError(err) + globalProj, err := generateProjectAllowList(resourceList, clusterRoleFileName, projName) + errors.CheckError(err) yamlBytes, err := yaml.Marshal(globalProj) errors.CheckError(err) @@ -78,26 +81,38 @@ func NewProjectAllowListGenCommand() *cobra.Command { return command } -func getResourceList(clientConfig clientcmd.ClientConfig) []*metav1.APIResourceList { +func getResourceList(clientConfig clientcmd.ClientConfig) ([]*metav1.APIResourceList, error) { config, err := clientConfig.ClientConfig() - errors.CheckError(err) + if err != nil { + return nil, err + } disco, err := discovery.NewDiscoveryClientForConfig(config) - errors.CheckError(err) + if err != nil { + return nil, err + } serverResources, err := disco.ServerPreferredResources() - errors.CheckError(err) - return serverResources + if err != nil { + return nil, err + } + return serverResources, nil } -func generateProjectAllowList(serverResources []*metav1.APIResourceList, clusterRoleFileName string, projName string) v1alpha1.AppProject { +func generateProjectAllowList(serverResources []*metav1.APIResourceList, clusterRoleFileName string, projName string) (*v1alpha1.AppProject, error) { yamlBytes, err := ioutil.ReadFile(clusterRoleFileName) - errors.CheckError(err) + if err != nil { + return nil, err + } var obj unstructured.Unstructured err = yaml.Unmarshal(yamlBytes, &obj) - errors.CheckError(err) + if err != nil { + return nil, err + } clusterRole := &rbacv1.ClusterRole{} err = scheme.Scheme.Convert(&obj, clusterRole, nil) - errors.CheckError(err) + if err != nil { + return nil, err + } resourceList := make([]metav1.GroupKind, 0) for _, rule := range clusterRole.Rules { @@ -143,5 +158,5 @@ func generateProjectAllowList(serverResources []*metav1.APIResourceList, cluster Spec: v1alpha1.AppProjectSpec{}, } globalProj.Spec.NamespaceResourceWhitelist = resourceList - return globalProj + return &globalProj, nil } diff --git a/cmd/argocd/commands/admin/project_allowlist_test.go b/cmd/argocd/commands/admin/project_allowlist_test.go index bb370e742bd44..c4634fb9310c1 100644 --- a/cmd/argocd/commands/admin/project_allowlist_test.go +++ b/cmd/argocd/commands/admin/project_allowlist_test.go @@ -14,6 +14,7 @@ func TestProjectAllowListGen(t *testing.T) { } resourceList := []*metav1.APIResourceList{{APIResources: []metav1.APIResource{res}}} - globalProj := generateProjectAllowList(resourceList, "testdata/test_clusterrole.yaml", "testproj") + globalProj, err := generateProjectAllowList(resourceList, "testdata/test_clusterrole.yaml", "testproj") + assert.NoError(t, err) assert.True(t, len(globalProj.Spec.NamespaceResourceWhitelist) > 0) } From 5a3f8d504b55fa8500fdf6c50d14cee2f557fc0e Mon Sep 17 00:00:00 2001 From: Michael Crenshaw Date: Sun, 10 Apr 2022 10:58:04 -0400 Subject: [PATCH 4/5] chore: better error messages Signed-off-by: Michael Crenshaw --- cmd/argocd/commands/admin/project_allowlist.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/cmd/argocd/commands/admin/project_allowlist.go b/cmd/argocd/commands/admin/project_allowlist.go index 835ae02cd2ff1..a465876461660 100644 --- a/cmd/argocd/commands/admin/project_allowlist.go +++ b/cmd/argocd/commands/admin/project_allowlist.go @@ -2,6 +2,7 @@ package admin import ( "bufio" + "fmt" "io" "io/ioutil" "os" @@ -84,15 +85,15 @@ func NewProjectAllowListGenCommand() *cobra.Command { func getResourceList(clientConfig clientcmd.ClientConfig) ([]*metav1.APIResourceList, error) { config, err := clientConfig.ClientConfig() if err != nil { - return nil, err + return nil, fmt.Errorf("error while creating client config: %s", err) } disco, err := discovery.NewDiscoveryClientForConfig(config) if err != nil { - return nil, err + return nil, fmt.Errorf("error while creating discovery client: %s", err) } serverResources, err := disco.ServerPreferredResources() if err != nil { - return nil, err + return nil, fmt.Errorf("error while getting server resources: %s", err) } return serverResources, nil } From 94733bc3fd3d2be140f48711639f8d52a94f643b Mon Sep 17 00:00:00 2001 From: Michael Crenshaw Date: Sun, 10 Apr 2022 11:01:01 -0400 Subject: [PATCH 5/5] chore: better error messages Signed-off-by: Michael Crenshaw --- cmd/argocd/commands/admin/project_allowlist.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/cmd/argocd/commands/admin/project_allowlist.go b/cmd/argocd/commands/admin/project_allowlist.go index a465876461660..0860e62d0b777 100644 --- a/cmd/argocd/commands/admin/project_allowlist.go +++ b/cmd/argocd/commands/admin/project_allowlist.go @@ -101,18 +101,18 @@ func getResourceList(clientConfig clientcmd.ClientConfig) ([]*metav1.APIResource func generateProjectAllowList(serverResources []*metav1.APIResourceList, clusterRoleFileName string, projName string) (*v1alpha1.AppProject, error) { yamlBytes, err := ioutil.ReadFile(clusterRoleFileName) if err != nil { - return nil, err + return nil, fmt.Errorf("error reading cluster role file: %s", err) } var obj unstructured.Unstructured err = yaml.Unmarshal(yamlBytes, &obj) if err != nil { - return nil, err + return nil, fmt.Errorf("error unmarshalling cluster role file yaml: %s", err) } clusterRole := &rbacv1.ClusterRole{} err = scheme.Scheme.Convert(&obj, clusterRole, nil) if err != nil { - return nil, err + return nil, fmt.Errorf("error converting cluster role yaml into ClusterRole struct: %s", err) } resourceList := make([]metav1.GroupKind, 0)