From 6ad15dcdf2539c627ae04a6bce4fa5484bdabd31 Mon Sep 17 00:00:00 2001 From: Mike Fedosin Date: Wed, 23 Feb 2022 14:25:54 +0100 Subject: [PATCH] Allow to define several API groups Currently approver can work with just one API group, which is defined by --apigroup option. This patch allows to set several API groups, so that the component is able to approve certificates for machines created by various APIs. Deprecations: --apigroup is deprecated in favor of --apigroupversion option New options: --api-group-version allows to specify both API group and version. This option can be given multiple times. --- go.mod | 2 +- main.go | 72 ++++++++++++++++++++--- manifests/04-deployment.yaml | 1 + pkg/controller/controller.go | 19 +++--- pkg/machinehandler/machinehandler.go | 23 ++++---- pkg/machinehandler/machinehandler_test.go | 4 +- 6 files changed, 94 insertions(+), 27 deletions(-) diff --git a/go.mod b/go.mod index d67fb1c7c..99eefeef7 100644 --- a/go.mod +++ b/go.mod @@ -10,6 +10,7 @@ require ( github.com/openshift/client-go v0.0.0-20211209144617-7385dd6338e3 github.com/openshift/library-go v0.0.0-20211217094823-cbb293285c81 github.com/prometheus/client_golang v1.11.0 + github.com/spf13/pflag v1.0.5 k8s.io/api v0.23.0 k8s.io/apimachinery v0.23.0 k8s.io/client-go v0.23.0 @@ -44,7 +45,6 @@ require ( github.com/prometheus/common v0.28.0 // indirect github.com/prometheus/procfs v0.6.0 // indirect github.com/robfig/cron v1.2.0 // indirect - github.com/spf13/pflag v1.0.5 // indirect golang.org/x/net v0.0.0-20210825183410-e898025ed96a // indirect golang.org/x/oauth2 v0.0.0-20210819190943-2bc19b11175f // indirect golang.org/x/sys v0.0.0-20211029165221-6e7872819dc8 // indirect diff --git a/main.go b/main.go index 97155b1b9..19be7c2aa 100644 --- a/main.go +++ b/main.go @@ -17,17 +17,20 @@ limitations under the License. package main import ( - "flag" + goflag "flag" "fmt" "os" "strconv" + "strings" "time" configv1 "github.com/openshift/api/config/v1" networkv1 "github.com/openshift/api/network/v1" "github.com/openshift/cluster-machine-approver/pkg/controller" "github.com/openshift/cluster-machine-approver/pkg/metrics" + flag "github.com/spf13/pflag" corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/client-go/rest" "k8s.io/client-go/tools/clientcmd" "k8s.io/client-go/tools/leaderelection/resourcelock" @@ -45,7 +48,8 @@ const ( func main() { var cliConfig string - var APIGroup string + var apiGroupVersions []string + var apiGroup string // deprecated var managementKubeConfigPath string var machineNamespace string var workloadKubeConfigPath string @@ -59,9 +63,11 @@ func main() { flagSet := flag.NewFlagSet("cluster-machine-approver", flag.ExitOnError) - klog.InitFlags(flagSet) + klog.InitFlags(nil) + flagSet.AddGoFlagSet(goflag.CommandLine) + flagSet.StringVar(&cliConfig, "config", "", "CLI config") - flagSet.StringVar(&APIGroup, "apigroup", "machine.openshift.io", "API group for machines, defaults to machine.openshift.io") + flagSet.StringSliceVar(&apiGroupVersions, "api-group-version", nil, "API group and version for machines in format '/'. If version is omitted, it will be set to the latest registered version in the cluster. Defaults to 'machine.openshift.io'. This option can be given multiple times.") flagSet.StringVar(&managementKubeConfigPath, "management-cluster-kubeconfig", "", "management kubeconfig path,") flagSet.StringVar(&machineNamespace, "machine-namespace", "", "restrict machine operations to a specific namespace, if not set, all machines will be observed in approval decisions") flagSet.StringVar(&workloadKubeConfigPath, "workload-cluster-kubeconfig", "", "workload kubeconfig path") @@ -75,8 +81,42 @@ func main() { flagSet.StringVar(&leaderElectResourceNamespace, "leader-elect-resource-namespace", "openshift-cluster-machine-approver", "the namespace in which the leader election resource will be created.") flagSet.Parse(os.Args[1:]) - if err := validateAPIGroup(APIGroup); err != nil { - klog.Fatalf(err.Error()) + // Deprecated options + flagSet.StringVar(&apiGroup, "apigroup", "", "API group for machines") + flagSet.MarkDeprecated("apigroup", "apigroup has been deprecated in favor of api-group-version option") + if apiGroup != "" && len(apiGroupVersions) > 0 { + klog.Fatal("Cannot set both --apigroup and --api-group-version options together.") + } + + var parsedAPIGroupVersions []schema.GroupVersion + + if len(apiGroupVersions) > 0 { + // Parsing API Group Versions + for _, apiGroupVersion := range apiGroupVersions { + parsedAPIGroupVersion, err := parseGroupVersion(apiGroupVersion) + if err != nil { + klog.Fatalf("Invalid API Group Version value: %s", apiGroupVersion) + } + parsedAPIGroupVersions = append(parsedAPIGroupVersions, parsedAPIGroupVersion) + } + } else { + // Setting the default + parsedAPIGroupVersions = []schema.GroupVersion{ + {Group: mapiGroup}, + } + } + + if apiGroup != "" { + // For backward compatibility with --apigroup option + parsedAPIGroupVersions = []schema.GroupVersion{ + {Group: apiGroup}, + } + } + + for _, parsedAPIGroupVersion := range parsedAPIGroupVersions { + if err := validateAPIGroup(parsedAPIGroupVersion.Group); err != nil { + klog.Fatalf(err.Error()) + } } // Now let's start the controller @@ -164,7 +204,7 @@ func main() { NodeClient: uncachedWorkloadClient, NodeRestCfg: workloadConfig, Config: controller.LoadConfig(cliConfig), - APIGroup: APIGroup, + APIGroupVersions: parsedAPIGroupVersions, }).SetupWithManager(mgr, ctrl.Options{}); err != nil { klog.Fatalf("unable to create CSR controller: %v", err) } @@ -225,3 +265,21 @@ func validateAPIGroup(apiGroup string) error { return nil } + +// parseGroupVersion turns "group/version" string into a GroupVersion struct. It reports error +// if it cannot parse the string. +func parseGroupVersion(gv string) (schema.GroupVersion, error) { + if (len(gv) == 0) || (gv == "/") { + return schema.GroupVersion{}, nil + } + + switch strings.Count(gv, "/") { + case 0: + return schema.GroupVersion{Group: gv}, nil + case 1: + i := strings.Index(gv, "/") + return schema.GroupVersion{Group: gv[:i], Version: gv[i+1:]}, nil + default: + return schema.GroupVersion{}, fmt.Errorf("unexpected GroupVersion string: %v", gv) + } +} diff --git a/manifests/04-deployment.yaml b/manifests/04-deployment.yaml index 50f1f5134..7c58431ea 100644 --- a/manifests/04-deployment.yaml +++ b/manifests/04-deployment.yaml @@ -68,6 +68,7 @@ spec: - "--leader-elect-renew-deadline=107s" - "--leader-elect-retry-period=26s" - "--leader-elect-resource-namespace=openshift-cluster-machine-approver" + - "--api-group-version=machine.openshift.io/v1beta1" resources: requests: memory: 50Mi diff --git a/pkg/controller/controller.go b/pkg/controller/controller.go index ecca0e5e9..413a2f7ff 100644 --- a/pkg/controller/controller.go +++ b/pkg/controller/controller.go @@ -12,6 +12,7 @@ import ( corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/runtime/schema" certificatesv1client "k8s.io/client-go/kubernetes/typed/certificates/v1" "k8s.io/client-go/rest" "k8s.io/klog/v2" @@ -40,8 +41,8 @@ type CertificateApprover struct { MachineRestCfg *rest.Config MachineNamespace string - Config ClusterMachineApproverConfig - APIGroup string + Config ClusterMachineApproverConfig + APIGroupVersions []schema.GroupVersion } func (m *CertificateApprover) SetupWithManager(mgr ctrl.Manager, options controller.Options) error { @@ -125,14 +126,18 @@ func (m *CertificateApprover) Reconcile(ctx context.Context, req ctrl.Request) ( Client: m.MachineClient, Config: m.MachineRestCfg, Ctx: ctx, - APIGroup: m.APIGroup, Namespace: m.MachineNamespace, } - machines, err := machineHandler.ListMachines() - if err != nil { - klog.Errorf("%v: Failed to list machines: %v", req.Name, err) - return reconcile.Result{}, fmt.Errorf("Failed to list machines: %w", err) + var machines []machinehandlerpkg.Machine + + for _, apiGroupVersion := range m.APIGroupVersions { + newMachines, err := machineHandler.ListMachines(apiGroupVersion) + if err != nil { + klog.Errorf("%v: Failed to list machines in API group %v: %v", req.Name, apiGroupVersion, err) + return reconcile.Result{}, fmt.Errorf("Failed to list machines: %w", err) + } + machines = append(machines, newMachines...) } nodes := &corev1.NodeList{} diff --git a/pkg/machinehandler/machinehandler.go b/pkg/machinehandler/machinehandler.go index 00897b4ab..cf1082498 100644 --- a/pkg/machinehandler/machinehandler.go +++ b/pkg/machinehandler/machinehandler.go @@ -21,7 +21,6 @@ var ( ) type MachineHandler struct { - APIGroup string Client client.Client Config *rest.Config Ctx context.Context @@ -38,17 +37,21 @@ type MachineStatus struct { } // ListMachines list all machines using given client -func (m *MachineHandler) ListMachines() ([]Machine, error) { - APIVersion, err := m.getAPIGroupPreferredVersion() - if err != nil { - return nil, err +func (m *MachineHandler) ListMachines(apiGroupVersion schema.GroupVersion) ([]Machine, error) { + apiVersion := apiGroupVersion.Version + if apiVersion == "" { + var err error + apiVersion, err = m.getAPIGroupPreferredVersion(apiGroupVersion.Group) + if err != nil { + return nil, err + } } unstructuredMachineList := &unstructured.UnstructuredList{} unstructuredMachineList.SetGroupVersionKind(schema.GroupVersionKind{ - Group: m.APIGroup, + Group: apiGroupVersion.Group, Kind: "MachineList", - Version: APIVersion, + Version: apiVersion, }) listOpts := make([]client.ListOption, 0) if m.Namespace != "" { @@ -89,7 +92,7 @@ func (m *MachineHandler) ListMachines() ([]Machine, error) { } // getAPIGroupPreferredVersion get preferred API version using API group -func (m *MachineHandler) getAPIGroupPreferredVersion() (string, error) { +func (m *MachineHandler) getAPIGroupPreferredVersion(apiGroup string) (string, error) { if m.Config == nil { return "", fmt.Errorf("machine handler config can't be nil") } @@ -105,12 +108,12 @@ func (m *MachineHandler) getAPIGroupPreferredVersion() (string, error) { } for _, group := range groupList.Groups { - if group.Name == m.APIGroup { + if group.Name == apiGroup { return group.PreferredVersion.Version, nil } } - return "", fmt.Errorf("failed to find API group %q", m.APIGroup) + return "", fmt.Errorf("failed to find API group %q", apiGroup) } // FindMatchingMachineFromInternalDNS find matching machine for node using internal DNS diff --git a/pkg/machinehandler/machinehandler_test.go b/pkg/machinehandler/machinehandler_test.go index 8b5ce7d84..a6b947af3 100644 --- a/pkg/machinehandler/machinehandler_test.go +++ b/pkg/machinehandler/machinehandler_test.go @@ -9,6 +9,7 @@ import ( "testing" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/client-go/rest" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/client/fake" @@ -155,13 +156,12 @@ func Test_authorizeCSR(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { handler := MachineHandler{ - APIGroup: tt.args.apiGroup, Client: tt.args.client, Config: tt.args.config, Ctx: tt.args.ctx, Namespace: tt.args.namespace, } - machines, err := handler.ListMachines() + machines, err := handler.ListMachines(schema.GroupVersion{Group: tt.args.apiGroup}) if (err != nil) != tt.wantErr { t.Errorf("unexpected error returned. wantErr: %t. err: %v.", tt.wantErr, err) }