diff --git a/cmd/controller-manager/app/controllermanager.go b/cmd/controller-manager/app/controllermanager.go index f01cfba12..a2d846596 100644 --- a/cmd/controller-manager/app/controllermanager.go +++ b/cmd/controller-manager/app/controllermanager.go @@ -96,6 +96,9 @@ func Run(ctx context.Context, opts *options.Options) error { return nil } +// setupControllers initialize controllers and setup one by one. +// Note: ignore cyclomatic complexity check(by gocyclo) because it will not effect readability. +//nolint:gocyclo func setupControllers(mgr controllerruntime.Manager, opts *options.Options, stopChan <-chan struct{}) { resetConfig := mgr.GetConfig() dynamicClientSet := dynamic.NewForConfigOrDie(resetConfig) @@ -103,8 +106,11 @@ func setupControllers(mgr controllerruntime.Manager, opts *options.Options, stop objectWatcher := objectwatcher.NewObjectWatcher(mgr.GetClient(), mgr.GetRESTMapper(), util.NewClusterDynamicClientSet) overrideManager := overridemanager.New(mgr.GetClient()) skippedResourceConfig := util.NewSkippedResourceConfig() - // TODO(pigletfly): add SkippedPropagatingAPIs validation - skippedResourceConfig.Parse(opts.SkippedPropagatingAPIs) + if err := skippedResourceConfig.Parse(opts.SkippedPropagatingAPIs); err != nil { + // TODO(RainbowMango): the parameter should be checked earlier. + // Consider add validation to 'options.Options' + panic(err) + } skippedPropagatingNamespaces := map[string]struct{}{} for _, ns := range opts.SkippedPropagatingNamespaces { skippedPropagatingNamespaces[ns] = struct{}{} diff --git a/pkg/util/apigroup.go b/pkg/util/apigroup.go index 140b12e5f..c54a557d4 100644 --- a/pkg/util/apigroup.go +++ b/pkg/util/apigroup.go @@ -1,10 +1,10 @@ package util import ( + "fmt" "strings" "k8s.io/apimachinery/pkg/runtime/schema" - "k8s.io/klog/v2" clusterv1alpha1 "github.com/karmada-io/karmada/pkg/apis/cluster/v1alpha1" policyv1alpha1 "github.com/karmada-io/karmada/pkg/apis/policy/v1alpha1" @@ -35,34 +35,36 @@ func NewSkippedResourceConfig() *SkippedResourceConfig { return r } -// Parse to parse tokens to SkippedResourceConfig -func (r *SkippedResourceConfig) Parse(c string) { - // v1/Node,Pod;networking.k8s.io/v1beta1/Ingress,IngressClass - // group networking.k8s.io - // group version networking.k8s.io/v1 - // group version kind networking.k8s.io/v1/Ingress - // corev1 has no group +// Parse parses the --skipped-propagating-apis input. +func (r *SkippedResourceConfig) Parse(c string) error { + // default(empty) input if c == "" { - return + return nil } tokens := strings.Split(c, ";") for _, token := range tokens { - r.parseSingle(token) + if err := r.parseSingle(token); err != nil { + return fmt.Errorf("parse --skipped-propagating-apis %w", err) + } } + + return nil } -func (r *SkippedResourceConfig) parseSingle(token string) { +func (r *SkippedResourceConfig) parseSingle(token string) error { switch strings.Count(token, "/") { + // Assume user don't want to skip the 'core'(no group name) group. + // So, it should be the case "". case 0: - // Group r.Groups[token] = struct{}{} + // it should be the case "/" case 1: + // for core group which don't have the group name, the case should be "v1/" or "v1/,..." if strings.HasPrefix(token, "v1") { - // core v1 - kinds := []string{} + var kinds []string for _, k := range strings.Split(token, ",") { - if strings.Contains(k, "/") { + if strings.Contains(k, "/") { // "v1/" s := strings.Split(k, "/") kinds = append(kinds, s[1]) } else { @@ -76,20 +78,22 @@ func (r *SkippedResourceConfig) parseSingle(token string) { } r.GroupVersionKinds[gvk] = struct{}{} } - } else { - // GroupVersion - i := strings.Index(token, "/") + } else { // case "/" + parts := strings.Split(token, "/") + if len(parts) != 2 { + return fmt.Errorf("invalid token: %s", token) + } gv := schema.GroupVersion{ - Group: token[:i], - Version: token[i+1:], + Group: parts[0], + Version: parts[1], } r.GroupVersions[gv] = struct{}{} } + // parameter format: "//" or "//,..." case 2: - // GroupVersionKind g := "" v := "" - kinds := []string{} + var kinds []string for _, k := range strings.Split(token, ",") { if strings.Contains(k, "/") { s := strings.Split(k, "/") @@ -109,8 +113,10 @@ func (r *SkippedResourceConfig) parseSingle(token string) { r.GroupVersionKinds[gvk] = struct{}{} } default: - klog.Error("Unsupported SkippedPropagatingAPIs: ", token) + return fmt.Errorf("invalid parameter: %s", token) } + + return nil } // GroupVersionDisabled returns whether GroupVersion is disabled. diff --git a/pkg/util/apigroup_test.go b/pkg/util/apigroup_test.go index 0c86789ba..16cf1c2af 100644 --- a/pkg/util/apigroup_test.go +++ b/pkg/util/apigroup_test.go @@ -11,32 +11,36 @@ func TestSkippedResourceConfigGVKParse(t *testing.T) { input string out []schema.GroupVersionKind }{ - {input: "v1/Node,Pod;networking.k8s.io/v1beta1/Ingress,IngressClass", out: []schema.GroupVersionKind{ - { - Group: "", - Version: "v1", - Kind: "Node", - }, - { - Group: "", - Version: "v1", - Kind: "Pod", - }, - { - Group: "networking.k8s.io", - Version: "v1beta1", - Kind: "Ingress", - }, - { - Group: "networking.k8s.io", - Version: "v1beta1", - Kind: "IngressClass", - }, - }}, + { + input: "v1/Node,Pod;networking.k8s.io/v1beta1/Ingress,IngressClass", + out: []schema.GroupVersionKind{ + { + Group: "", + Version: "v1", + Kind: "Node", + }, + { + Group: "", + Version: "v1", + Kind: "Pod", + }, + { + Group: "networking.k8s.io", + Version: "v1beta1", + Kind: "Ingress", + }, + { + Group: "networking.k8s.io", + Version: "v1beta1", + Kind: "IngressClass", + }, + }}, } for _, test := range tests { r := NewSkippedResourceConfig() - r.Parse(test.input) + if err := r.Parse(test.input); err != nil { + t.Fatalf("Unexpected error: %v", err) + } for i, o := range test.out { ok := r.GroupVersionKindDisabled(o) if !ok { @@ -50,28 +54,32 @@ func TestResourceConfigGVParse(t *testing.T) { input string out []schema.GroupVersion }{ - {input: "networking.k8s.io/v1;test/v1beta1", out: []schema.GroupVersion{ - { - Group: "networking.k8s.io", - Version: "v1", - }, - { - Group: "networking.k8s.io", - Version: "v1", - }, - { - Group: "test", - Version: "v1beta1", - }, - { - Group: "test", - Version: "v1beta1", - }, - }}, + { + input: "networking.k8s.io/v1;test/v1beta1", + out: []schema.GroupVersion{ + { + Group: "networking.k8s.io", + Version: "v1", + }, + { + Group: "networking.k8s.io", + Version: "v1", + }, + { + Group: "test", + Version: "v1beta1", + }, + { + Group: "test", + Version: "v1beta1", + }, + }}, } for _, test := range tests { r := NewSkippedResourceConfig() - r.Parse(test.input) + if err := r.Parse(test.input); err != nil { + t.Fatalf("Unexpected error: %v", err) + } for i, o := range test.out { ok := r.GroupVersionDisabled(o) if !ok { @@ -86,13 +94,17 @@ func TestSkippedResourceConfigGroupParse(t *testing.T) { input string out []string }{ - {input: "networking.k8s.io;test", out: []string{ - "networking.k8s.io", "test", - }}, + { + input: "networking.k8s.io;test", + out: []string{ + "networking.k8s.io", "test", + }}, } for _, test := range tests { r := NewSkippedResourceConfig() - r.Parse(test.input) + if err := r.Parse(test.input); err != nil { + t.Fatalf("Unexpected error: %v", err) + } for i, o := range test.out { ok := r.GroupDisabled(o) if !ok { @@ -107,43 +119,47 @@ func TestSkippedResourceConfigMixedParse(t *testing.T) { input string out SkippedResourceConfig }{ - {input: "v1/Node,Pod;networking.k8s.io/v1beta1/Ingress,IngressClass;networking.k8s.io;test.com/v1", out: SkippedResourceConfig{ - Groups: map[string]struct{}{ - "networking.k8s.io": {}, - }, - GroupVersions: map[schema.GroupVersion]struct{}{ - { - Group: "test.com", - Version: "v1", - }: {}, - }, - GroupVersionKinds: map[schema.GroupVersionKind]struct{}{ - { - Group: "", - Version: "v1", - Kind: "Node", - }: {}, - { - Group: "", - Version: "v1", - Kind: "Pod", - }: {}, - { - Group: "networking.k8s.io", - Version: "v1beta1", - Kind: "Ingress", - }: {}, - { - Group: "networking.k8s.io", - Version: "v1beta1", - Kind: "IngressClass", - }: {}, - }, - }}, + { + input: "v1/Node,Pod;networking.k8s.io/v1beta1/Ingress,IngressClass;networking.k8s.io;test.com/v1", + out: SkippedResourceConfig{ + Groups: map[string]struct{}{ + "networking.k8s.io": {}, + }, + GroupVersions: map[schema.GroupVersion]struct{}{ + { + Group: "test.com", + Version: "v1", + }: {}, + }, + GroupVersionKinds: map[schema.GroupVersionKind]struct{}{ + { + Group: "", + Version: "v1", + Kind: "Node", + }: {}, + { + Group: "", + Version: "v1", + Kind: "Pod", + }: {}, + { + Group: "networking.k8s.io", + Version: "v1beta1", + Kind: "Ingress", + }: {}, + { + Group: "networking.k8s.io", + Version: "v1beta1", + Kind: "IngressClass", + }: {}, + }, + }}, } for i, test := range tests { r := NewSkippedResourceConfig() - r.Parse(test.input) + if err := r.Parse(test.input); err != nil { + t.Fatalf("Unexpected error: %v", err) + } for g := range r.Groups { ok := r.GroupDisabled(g) if !ok { @@ -172,13 +188,17 @@ func TestDefaultSkippedResourceConfigGroupParse(t *testing.T) { input string out []string }{ - {input: "", out: []string{ - "cluster.karmada.io", "policy.karmada.io", "work.karmada.io", - }}, + { + input: "", + out: []string{ + "cluster.karmada.io", "policy.karmada.io", "work.karmada.io", + }}, } for _, test := range tests { r := NewSkippedResourceConfig() - r.Parse(test.input) + if err := r.Parse(test.input); err != nil { + t.Fatalf("Unexpected error: %v", err) + } for i, o := range test.out { ok := r.GroupDisabled(o) if !ok {