Allow to define several API groups

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

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 '<group>/<version' or just '<group>'. 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)
+	}
+}

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

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{}

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

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)
 			}