diff --git a/pkg/operators/klusterlet/controllers/ssarcontroller/klusterlet_ssar_controller.go b/pkg/operators/klusterlet/controllers/ssarcontroller/klusterlet_ssar_controller.go new file mode 100644 index 000000000..1e2eaac98 --- /dev/null +++ b/pkg/operators/klusterlet/controllers/ssarcontroller/klusterlet_ssar_controller.go @@ -0,0 +1,435 @@ +package ssarcontroller + +import ( + "context" + "fmt" + "strings" + "sync" + + authorizationv1 "k8s.io/api/authorization/v1" + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/api/meta" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + coreinformer "k8s.io/client-go/informers/core/v1" + "k8s.io/client-go/kubernetes" + corelister "k8s.io/client-go/listers/core/v1" + "k8s.io/klog/v2" + + "github.com/openshift/library-go/pkg/controller/factory" + "github.com/openshift/library-go/pkg/operator/events" + + operatorv1client "open-cluster-management.io/api/client/operator/clientset/versioned/typed/operator/v1" + operatorinformer "open-cluster-management.io/api/client/operator/informers/externalversions/operator/v1" + operatorlister "open-cluster-management.io/api/client/operator/listers/operator/v1" + "open-cluster-management.io/registration-operator/pkg/helpers" +) + +type ssarController struct { + kubeClient kubernetes.Interface + secretLister corelister.SecretLister + klusterletClient operatorv1client.KlusterletInterface + klusterletLister operatorlister.KlusterletLister + *klusterletLocker +} + +type klusterletLocker struct { + sync.RWMutex + klusterletInChecking map[string]struct{} +} + +const ( + klusterletNamespace = "open-cluster-management-agent" + bootstrapSecret = "BootstrapSecret" + bootstrapSecretDegraded = "BootstrapSecretDegraded" + hubConfigSecret = "HubConfigSecret" + hubConfigSecretDegraded = "HubConfigSecretDegraded" +) + +func NewKlustrletSSARController( + kubeClient kubernetes.Interface, + klusterletClient operatorv1client.KlusterletInterface, + klusterletInformer operatorinformer.KlusterletInformer, + secretInformer coreinformer.SecretInformer, + recorder events.Recorder, +) factory.Controller { + controller := &ssarController{ + kubeClient: kubeClient, + klusterletClient: klusterletClient, + klusterletLister: klusterletInformer.Lister(), + secretLister: secretInformer.Lister(), + klusterletLocker: &klusterletLocker{ + klusterletInChecking: make(map[string]struct{}), + }, + } + + return factory.New().WithSync(controller.sync). + WithInformersQueueKeyFunc(helpers.KlusterletSecretQueueKeyFunc(controller.klusterletLister), secretInformer.Informer()). + WithInformersQueueKeyFunc(func(obj runtime.Object) string { + accessor, _ := meta.Accessor(obj) + return accessor.GetName() + }, klusterletInformer.Informer()). + ToController("KlusterletSSARController", recorder) +} + +func (l *klusterletLocker) inSSARChecking(klusterletName string) bool { + l.RLock() + defer l.RUnlock() + _, ok := l.klusterletInChecking[klusterletName] + return ok +} + +func (l *klusterletLocker) addSSARChecking(klusterletName string) { + l.Lock() + defer l.Unlock() + l.klusterletInChecking[klusterletName] = struct{}{} +} + +func (l *klusterletLocker) deleteSSARChecking(klusterletName string) { + l.Lock() + defer l.Unlock() + delete(l.klusterletInChecking, klusterletName) +} + +func (c *ssarController) sync(ctx context.Context, controllerContext factory.SyncContext) error { + klusterletName := controllerContext.QueueKey() + if klusterletName == "" { + return nil + } + klusterlet, err := c.klusterletLister.Get(klusterletName) + switch { + case errors.IsNotFound(err): + return nil + case err != nil: + return err + } + klusterlet = klusterlet.DeepCopy() + + // if the ssar checking is already processing, ignore reconciling this turn. + if c.inSSARChecking(klusterletName) { + klog.V(4).Info("Reconciling Klusterlet %q is already processing now", klusterletName) + return nil + } + + c.addSSARChecking(klusterletName) + go func() { + defer c.deleteSSARChecking(klusterletName) + + klog.V(4).Infof("Reconciling Klusterlet %q", klusterletName) + klusterletNS := klusterlet.Spec.Namespace + if klusterletNS == "" { + klusterletNS = klusterletNamespace + } + + bootstrapDegradedCondition := checkAgentDegradedCondition( + ctx, c.kubeClient, + bootstrapSecret, bootstrapSecretDegraded, + klusterletAgent{ + clusterName: klusterlet.Spec.ClusterName, + namespace: klusterletNS, + }, + []degradedCheckFunc{checkBootstrapSecret}, + ) + + hubConfigDegradedCondition := checkAgentDegradedCondition( + ctx, c.kubeClient, + hubConfigSecret, hubConfigSecretDegraded, + klusterletAgent{ + clusterName: klusterlet.Spec.ClusterName, + namespace: klusterletNS, + }, + []degradedCheckFunc{checkHubConfigSecret}, + ) + + _, _, err = helpers.UpdateKlusterletStatus(ctx, c.klusterletClient, klusterletName, + helpers.UpdateKlusterletConditionFn(bootstrapDegradedCondition), + helpers.UpdateKlusterletConditionFn(hubConfigDegradedCondition)) + + if err != nil { + klog.Errorf("Update Klusterlet Status Failed: %v", err) + } + }() + + return nil +} + +type klusterletAgent struct { + clusterName string + namespace string +} + +func checkAgentDegradedCondition( + ctx context.Context, kubeClient kubernetes.Interface, + secretName, degradedType string, + agent klusterletAgent, + degradedCheckFns []degradedCheckFunc) metav1.Condition { + degradedConditionReasons := []string{} + degradedConditionMessages := []string{} + for _, degradedCheckFn := range degradedCheckFns { + currCond := degradedCheckFn(ctx, kubeClient, agent) + if currCond == nil { + continue + } + degradedConditionReasons = append(degradedConditionReasons, currCond.Reason) + degradedConditionMessages = append(degradedConditionMessages, currCond.Message) + } + + if len(degradedConditionReasons) == 0 { + return metav1.Condition{ + Type: degradedType, + Status: metav1.ConditionFalse, + Reason: fmt.Sprintf("%sFunctional", secretName), + Message: fmt.Sprintf("%s is functioning correctly", secretName), + } + } + + return metav1.Condition{ + Type: degradedType, + Status: metav1.ConditionTrue, + Reason: strings.Join(degradedConditionReasons, ","), + Message: strings.Join(degradedConditionMessages, "\n"), + } +} + +type degradedCheckFunc func(ctx context.Context, kubeClient kubernetes.Interface, agent klusterletAgent) *metav1.Condition + +// Check bootstrap secret, if the secret is invalid, return registration degraded condition +func checkBootstrapSecret(ctx context.Context, kubeClient kubernetes.Interface, agent klusterletAgent) *metav1.Condition { + // Check if bootstrap secret exists + bootstrapSecret, err := kubeClient.CoreV1().Secrets(agent.namespace).Get(ctx, helpers.BootstrapHubKubeConfig, metav1.GetOptions{}) + if err != nil { + return &metav1.Condition{ + Reason: "BootstrapSecretMissing", + Message: fmt.Sprintf("Failed to get bootstrap secret %q %q: %v", agent.namespace, helpers.BootstrapHubKubeConfig, err), + } + } + + // Check if bootstrap secret works by building kube client + bootstrapClient, err := buildKubeClientWithSecret(bootstrapSecret) + if err != nil { + return &metav1.Condition{ + Reason: "BootstrapSecretError", + Message: fmt.Sprintf("Failed to build bootstrap kube client with bootstrap secret %q %q: %v", + agent.namespace, helpers.BootstrapHubKubeConfig, err), + } + } + + // Check the bootstrap client permissions by creating SelfSubjectAccessReviews + allowed, failedReview, err := createSelfSubjectAccessReviews(ctx, bootstrapClient, getBootstrapSSARs()) + if err != nil { + return &metav1.Condition{ + Reason: "BootstrapSecretError", + Message: fmt.Sprintf("Failed to create %+v with bootstrap secret %q %q: %v", + failedReview, agent.namespace, helpers.BootstrapHubKubeConfig, err), + } + } + if !allowed { + return &metav1.Condition{ + Reason: "BootstrapSecretUnauthorized", + Message: fmt.Sprintf("Operation for resource %+v is not allowed with bootstrap secret %q %q", + failedReview.Spec.ResourceAttributes, agent.namespace, helpers.BootstrapHubKubeConfig), + } + } + + return nil +} + +func getBootstrapSSARs() []authorizationv1.SelfSubjectAccessReview { + reviews := []authorizationv1.SelfSubjectAccessReview{} + clusterResource := authorizationv1.ResourceAttributes{ + Group: "cluster.open-cluster-management.io", + Resource: "managedclusters", + } + reviews = append(reviews, generateSelfSubjectAccessReviews(clusterResource, "create", "get")...) + + certResource := authorizationv1.ResourceAttributes{ + Group: "certificates.k8s.io", + Resource: "certificatesigningrequests", + } + return append(reviews, generateSelfSubjectAccessReviews(certResource, "create", "get", "list", "watch")...) +} + +// Check hub-kubeconfig-secret, if the secret is invalid, return degraded condition +func checkHubConfigSecret(ctx context.Context, kubeClient kubernetes.Interface, agent klusterletAgent) *metav1.Condition { + hubConfigSecret, err := kubeClient.CoreV1().Secrets(agent.namespace).Get(ctx, helpers.HubKubeConfig, metav1.GetOptions{}) + if err != nil { + return &metav1.Condition{ + Reason: "HubKubeConfigSecretMissing", + Message: fmt.Sprintf("Failed to get hub kubeconfig secret %q %q: %v", agent.namespace, helpers.HubKubeConfig, err), + } + } + + if hubConfigSecret.Data["kubeconfig"] == nil { + return &metav1.Condition{ + Reason: "HubKubeConfigMissing", + Message: fmt.Sprintf("Failed to get kubeconfig from `kubectl get secret -n %q %q -ojsonpath='{.data.kubeconfig}'`. "+ + "This is set by the klusterlet registration deployment, but the CSR must be approved by the cluster-admin on the hub.", + hubConfigSecret.Namespace, hubConfigSecret.Name), + } + } + + hubClient, err := buildKubeClientWithSecret(hubConfigSecret) + if err != nil { + return &metav1.Condition{ + Reason: "HubKubeConfigError", + Message: fmt.Sprintf("Failed to build hub kube client with hub config secret %q %q: %v", + hubConfigSecret.Namespace, hubConfigSecret.Name, err), + } + } + + clusterName := agent.clusterName + // If cluster name is empty, read cluster name from hub config secret + if clusterName == "" { + if hubConfigSecret.Data["cluster-name"] == nil { + return &metav1.Condition{ + Reason: "ClusterNameMissing", + Message: fmt.Sprintf( + "Failed to get cluster name from `kubectl get secret -n %q %q -ojsonpath='{.data.cluster-name}`."+ + " This is set by the klusterlet registration deployment.", hubConfigSecret.Namespace, hubConfigSecret.Name), + } + } + clusterName = string(hubConfigSecret.Data["cluster-name"]) + } + + // Check the hub kubeconfig permissions by creating SelfSubjectAccessReviews + allowed, failedReview, err := createSelfSubjectAccessReviews(ctx, hubClient, getHubConfigSSARs(clusterName)) + if err != nil { + return &metav1.Condition{ + Reason: "HubKubeConfigError", + Message: fmt.Sprintf("Failed to create %+v with hub config secret %q %q: %v", + failedReview, hubConfigSecret.Namespace, hubConfigSecret.Name, err), + } + } + if !allowed { + return &metav1.Condition{ + Reason: "HubKubeConfigUnauthorized", + Message: fmt.Sprintf("Operation for resource %+v is not allowed with hub config secret %q %q", + failedReview.Spec.ResourceAttributes, hubConfigSecret.Namespace, hubConfigSecret.Name), + } + } + + return nil +} + +func getHubConfigSSARs(clusterName string) []authorizationv1.SelfSubjectAccessReview { + reviews := []authorizationv1.SelfSubjectAccessReview{} + + // registration resources + certResource := authorizationv1.ResourceAttributes{ + Group: "certificates.k8s.io", + Resource: "certificatesigningrequests", + } + reviews = append(reviews, generateSelfSubjectAccessReviews(certResource, "get", "list", "watch")...) + + clusterResource := authorizationv1.ResourceAttributes{ + Group: "cluster.open-cluster-management.io", + Resource: "managedclusters", + Name: clusterName, + } + reviews = append(reviews, generateSelfSubjectAccessReviews(clusterResource, "get", "list", "update", "watch")...) + + clusterStatusResource := authorizationv1.ResourceAttributes{ + Group: "cluster.open-cluster-management.io", + Resource: "managedclusters", + Subresource: "status", + Name: clusterName, + } + reviews = append(reviews, generateSelfSubjectAccessReviews(clusterStatusResource, "patch", "update")...) + + clusterCertResource := authorizationv1.ResourceAttributes{ + Group: "register.open-cluster-management.io", + Resource: "managedclusters", + Subresource: "clientcertificates", + } + reviews = append(reviews, generateSelfSubjectAccessReviews(clusterCertResource, "renew")...) + + leaseResource := authorizationv1.ResourceAttributes{ + Group: "coordination.k8s.io", + Resource: "leases", + Name: fmt.Sprintf("cluster-lease-%s", clusterName), + Namespace: clusterName, + } + reviews = append(reviews, generateSelfSubjectAccessReviews(leaseResource, "get", "update")...) + + // work resources + eventResource := authorizationv1.ResourceAttributes{ + Resource: "events", + Namespace: clusterName, + } + reviews = append(reviews, generateSelfSubjectAccessReviews(eventResource, "create", "patch", "update")...) + + eventResource = authorizationv1.ResourceAttributes{ + Group: "events.k8s.io", + Resource: "events", + Namespace: clusterName, + } + reviews = append(reviews, generateSelfSubjectAccessReviews(eventResource, "create", "patch", "update")...) + + workResource := authorizationv1.ResourceAttributes{ + Group: "work.open-cluster-management.io", + Resource: "manifestworks", + Namespace: clusterName, + } + reviews = append(reviews, generateSelfSubjectAccessReviews(workResource, "get", "list", "watch", "update")...) + + workStatusResource := authorizationv1.ResourceAttributes{ + Group: "work.open-cluster-management.io", + Resource: "manifestworks", + Subresource: "status", + Namespace: clusterName, + } + reviews = append(reviews, generateSelfSubjectAccessReviews(workStatusResource, "patch", "update")...) + return reviews +} + +func buildKubeClientWithSecret(secret *corev1.Secret) (kubernetes.Interface, error) { + restConfig, err := helpers.LoadClientConfigFromSecret(secret) + if err != nil { + return nil, err + } + + // reduce qps and burst of client, because too many managed clusters registration on hub and send ssar requests at once could cause resource pressure + restConfig.QPS = 2 + restConfig.Burst = 5 + + return kubernetes.NewForConfig(restConfig) +} + +func generateSelfSubjectAccessReviews(resource authorizationv1.ResourceAttributes, verbs ...string) []authorizationv1.SelfSubjectAccessReview { + reviews := []authorizationv1.SelfSubjectAccessReview{} + for _, verb := range verbs { + reviews = append(reviews, authorizationv1.SelfSubjectAccessReview{ + Spec: authorizationv1.SelfSubjectAccessReviewSpec{ + ResourceAttributes: &authorizationv1.ResourceAttributes{ + Group: resource.Group, + Resource: resource.Resource, + Subresource: resource.Subresource, + Name: resource.Name, + Namespace: resource.Namespace, + Verb: verb, + }, + }, + }) + } + return reviews +} + +func createSelfSubjectAccessReviews( + ctx context.Context, + kubeClient kubernetes.Interface, + selfSubjectAccessReviews []authorizationv1.SelfSubjectAccessReview) (bool, *authorizationv1.SelfSubjectAccessReview, error) { + + for i := range selfSubjectAccessReviews { + subjectAccessReview := selfSubjectAccessReviews[i] + + ssar, err := kubeClient.AuthorizationV1().SelfSubjectAccessReviews().Create(ctx, &subjectAccessReview, metav1.CreateOptions{}) + if err != nil { + return false, &subjectAccessReview, err + } + if !ssar.Status.Allowed { + return false, &subjectAccessReview, nil + } + } + return true, nil, nil +} diff --git a/pkg/operators/klusterlet/controllers/ssarcontroller/klusterlet_ssar_controller_test.go b/pkg/operators/klusterlet/controllers/ssarcontroller/klusterlet_ssar_controller_test.go new file mode 100644 index 000000000..d55b13181 --- /dev/null +++ b/pkg/operators/klusterlet/controllers/ssarcontroller/klusterlet_ssar_controller_test.go @@ -0,0 +1,335 @@ +package ssarcontroller + +import ( + "context" + "encoding/json" + "io/ioutil" + "net/http" + "net/http/httptest" + "testing" + "time" + + authorizationv1 "k8s.io/api/authorization/v1" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + kubeinformers "k8s.io/client-go/informers" + fakekube "k8s.io/client-go/kubernetes/fake" + clienttesting "k8s.io/client-go/testing" + clientcmdapi "k8s.io/client-go/tools/clientcmd/api" + clientcmdlatest "k8s.io/client-go/tools/clientcmd/api/latest" + + fakeoperatorclient "open-cluster-management.io/api/client/operator/clientset/versioned/fake" + operatorinformers "open-cluster-management.io/api/client/operator/informers/externalversions" + operatorapiv1 "open-cluster-management.io/api/operator/v1" + "open-cluster-management.io/registration-operator/pkg/helpers" + testinghelper "open-cluster-management.io/registration-operator/pkg/helpers/testing" +) + +type testController struct { + controller *ssarController + operatorClient *fakeoperatorclient.Clientset +} + +type serverResponse struct { + allowToOperateManagedClusters bool + allowToOperateManagedClusterStatus bool + allowToOperateManifestWorks bool +} + +func newSecret(name, namespace string) *corev1.Secret { + return &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: namespace, + }, + Data: map[string][]byte{}, + } +} + +func newKubeConfig(host string) []byte { + configData, _ := runtime.Encode(clientcmdlatest.Codec, &clientcmdapi.Config{ + Clusters: map[string]*clientcmdapi.Cluster{"default-cluster": { + Server: host, + InsecureSkipTLSVerify: true, + }}, + Contexts: map[string]*clientcmdapi.Context{"default-context": { + Cluster: "default-cluster", + }}, + CurrentContext: "default-context", + }) + return configData +} + +func newSecretWithKubeConfig(name, namespace string, kubeConfig []byte) *corev1.Secret { + secret := newSecret(name, namespace) + secret.Data["kubeconfig"] = kubeConfig + return secret +} + +func newKlusterlet(name, namespace, clustername string) *operatorapiv1.Klusterlet { + return &operatorapiv1.Klusterlet{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + }, + Spec: operatorapiv1.KlusterletSpec{ + RegistrationImagePullSpec: "testregistration", + WorkImagePullSpec: "testwork", + ClusterName: clustername, + Namespace: namespace, + ExternalServerURLs: []operatorapiv1.ServerURL{}, + }, + } +} + +func newTestController(klusterlet *operatorapiv1.Klusterlet, objects ...runtime.Object) *testController { + fakeKubeClient := fakekube.NewSimpleClientset(objects...) + fakeOperatorClient := fakeoperatorclient.NewSimpleClientset(klusterlet) + operatorInformers := operatorinformers.NewSharedInformerFactory(fakeOperatorClient, 5*time.Minute) + kubeInformers := kubeinformers.NewSharedInformerFactory(fakeKubeClient, 5*time.Minute) + + klusterletController := &ssarController{ + kubeClient: fakeKubeClient, + klusterletClient: fakeOperatorClient.OperatorV1().Klusterlets(), + secretLister: kubeInformers.Core().V1().Secrets().Lister(), + klusterletLister: operatorInformers.Operator().V1().Klusterlets().Lister(), + klusterletLocker: &klusterletLocker{ + klusterletInChecking: make(map[string]struct{}), + }, + } + + store := operatorInformers.Operator().V1().Klusterlets().Informer().GetStore() + store.Add(klusterlet) + + return &testController{ + controller: klusterletController, + operatorClient: fakeOperatorClient, + } +} + +func TestSync(t *testing.T) { + response := &serverResponse{} + apiServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) { + if req.URL.Path != "/apis/authorization.k8s.io/v1/selfsubjectaccessreviews" { + w.WriteHeader(http.StatusNotFound) + return + } + + data, err := ioutil.ReadAll(req.Body) + if err != nil { + w.WriteHeader(http.StatusInternalServerError) + return + } + ssar := &authorizationv1.SelfSubjectAccessReview{} + json.Unmarshal(data, ssar) + if ssar.Spec.ResourceAttributes.Resource == "managedclusters" { + if ssar.Spec.ResourceAttributes.Subresource == "status" { + ssar.Status.Allowed = response.allowToOperateManagedClusterStatus + } else { + ssar.Status.Allowed = response.allowToOperateManagedClusters + } + } else if ssar.Spec.ResourceAttributes.Resource == "manifestworks" { + ssar.Status.Allowed = response.allowToOperateManifestWorks + } else { + ssar.Status.Allowed = true + } + + w.Header().Set("Content-type", "application/json") + w.WriteHeader(http.StatusCreated) + json.NewEncoder(w).Encode(ssar) + })) + defer apiServer.Close() + + apiServerHost := apiServer.URL + + cases := []struct { + name string + object []runtime.Object + klusterlet *operatorapiv1.Klusterlet + allowToOperateManagedClusters bool + allowToOperateManagedClusterStatus bool + allowToOperateManifestWorks bool + expectedConditions []metav1.Condition + }{ + { + name: "No bootstrap secret", + object: []runtime.Object{ + newSecretWithKubeConfig(helpers.HubKubeConfig, "test", newKubeConfig(apiServerHost)), + }, + klusterlet: newKlusterlet("testklusterlet", "test", "cluster1"), + allowToOperateManagedClusters: true, + allowToOperateManagedClusterStatus: true, + allowToOperateManifestWorks: true, + expectedConditions: []metav1.Condition{ + testinghelper.NamedCondition(bootstrapSecretDegraded, "BootstrapSecretMissing", metav1.ConditionTrue), + testinghelper.NamedCondition(hubConfigSecretDegraded, "HubConfigSecretFunctional", metav1.ConditionFalse), + }, + }, + { + name: "No hubconfig secret", + object: []runtime.Object{ + newSecretWithKubeConfig(helpers.BootstrapHubKubeConfig, "test", newKubeConfig(apiServerHost)), + }, + klusterlet: newKlusterlet("testklusterlet", "test", "cluster1"), + allowToOperateManagedClusters: true, + allowToOperateManagedClusterStatus: true, + allowToOperateManifestWorks: true, + expectedConditions: []metav1.Condition{ + testinghelper.NamedCondition(bootstrapSecretDegraded, "BootstrapSecretFunctional", metav1.ConditionFalse), + testinghelper.NamedCondition(hubConfigSecretDegraded, "HubKubeConfigSecretMissing", metav1.ConditionTrue), + }, + }, + { + name: "Bad bootstrap secret", + object: []runtime.Object{ + newSecretWithKubeConfig(helpers.BootstrapHubKubeConfig, "test", []byte("badsecret")), + newSecretWithKubeConfig(helpers.HubKubeConfig, "test", newKubeConfig(apiServerHost)), + }, + allowToOperateManagedClusters: true, + allowToOperateManagedClusterStatus: true, + allowToOperateManifestWorks: true, + klusterlet: newKlusterlet("testklusterlet", "test", "cluster1"), + expectedConditions: []metav1.Condition{ + testinghelper.NamedCondition(bootstrapSecretDegraded, "BootstrapSecretError", metav1.ConditionTrue), + testinghelper.NamedCondition(hubConfigSecretDegraded, "HubConfigSecretFunctional", metav1.ConditionFalse), + }, + }, + { + name: "Bad hub config secret", + object: []runtime.Object{ + newSecretWithKubeConfig(helpers.BootstrapHubKubeConfig, "test", newKubeConfig(apiServerHost)), + newSecretWithKubeConfig(helpers.HubKubeConfig, "test", []byte("badkubeconfig")), + }, + allowToOperateManagedClusters: true, + allowToOperateManagedClusterStatus: true, + allowToOperateManifestWorks: true, + klusterlet: newKlusterlet("testklusterlet", "test", "cluster1"), + expectedConditions: []metav1.Condition{ + testinghelper.NamedCondition(bootstrapSecretDegraded, "BootstrapSecretFunctional", metav1.ConditionFalse), + testinghelper.NamedCondition(hubConfigSecretDegraded, "HubKubeConfigError", metav1.ConditionTrue), + }, + }, + { + name: "Unauthorized", + object: []runtime.Object{ + newSecretWithKubeConfig(helpers.BootstrapHubKubeConfig, "test", newKubeConfig(apiServerHost)), + newSecretWithKubeConfig(helpers.HubKubeConfig, "test", newKubeConfig(apiServerHost)), + }, + allowToOperateManagedClusters: false, + allowToOperateManagedClusterStatus: false, + allowToOperateManifestWorks: false, + klusterlet: newKlusterlet("testklusterlet", "test", "cluster1"), + expectedConditions: []metav1.Condition{ + testinghelper.NamedCondition(bootstrapSecretDegraded, "BootstrapSecretUnauthorized", metav1.ConditionTrue), + testinghelper.NamedCondition(hubConfigSecretDegraded, "HubKubeConfigUnauthorized", metav1.ConditionTrue), + }, + }, + { + name: "Operator functional", + object: []runtime.Object{ + newSecretWithKubeConfig(helpers.BootstrapHubKubeConfig, "test", newKubeConfig(apiServerHost)), + newSecretWithKubeConfig(helpers.HubKubeConfig, "test", newKubeConfig(apiServerHost)), + }, + allowToOperateManagedClusters: true, + allowToOperateManagedClusterStatus: true, + allowToOperateManifestWorks: true, + klusterlet: newKlusterlet("testklusterlet", "test", "cluster1"), + expectedConditions: []metav1.Condition{ + testinghelper.NamedCondition(bootstrapSecretDegraded, "BootstrapSecretFunctional", metav1.ConditionFalse), + testinghelper.NamedCondition(hubConfigSecretDegraded, "HubConfigSecretFunctional", metav1.ConditionFalse), + }, + }, + } + + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + controller := newTestController(c.klusterlet, c.object...) + syncContext := testinghelper.NewFakeSyncContext(t, c.klusterlet.Name) + + response.allowToOperateManagedClusters = c.allowToOperateManagedClusters + response.allowToOperateManagedClusterStatus = c.allowToOperateManagedClusterStatus + response.allowToOperateManifestWorks = c.allowToOperateManifestWorks + + err := controller.controller.sync(context.TODO(), syncContext) + if err != nil { + t.Errorf("Expected no error when update status: %v", err) + } + + // wait util goroutine is done + for controller.controller.inSSARChecking(c.klusterlet.Name) { + time.Sleep(time.Second * 1) + } + + operatorActions := controller.operatorClient.Actions() + + testinghelper.AssertEqualNumber(t, len(operatorActions), 2) + testinghelper.AssertGet(t, operatorActions[0], "operator.open-cluster-management.io", "v1", "klusterlets") + testinghelper.AssertAction(t, operatorActions[1], "update") + testinghelper.AssertOnlyConditions(t, operatorActions[1].(clienttesting.UpdateActionImpl).Object, c.expectedConditions...) + }) + } +} + +func TestLocker(t *testing.T) { + locker := &klusterletLocker{ + klusterletInChecking: make(map[string]struct{}), + } + + cluster1 := "cluster1" + cluster2 := "cluster2" + results := make(chan string, 2) + + // first we add cluster1 in processing stage + c1status := locker.inSSARChecking(cluster1) + if c1status { + t.Error("c1 should not be processing yet") + } + + locker.addSSARChecking(cluster1) + + go func() { + defer locker.deleteSSARChecking(cluster1) + // wait for 5 seconds + time.Sleep(time.Second * 5) + results <- cluster1 + }() + + // then we add cluster2 in processing stage + go func() { + // begin when cluster1 is in processing + for !locker.inSSARChecking(cluster1) { + time.Sleep(time.Millisecond * 500) + } + + // simulate the controller part + c2status := locker.inSSARChecking(cluster2) + if c2status { + t.Error("c2 should not be processing yet") + } + + locker.addSSARChecking(cluster2) + + go func() { + defer locker.deleteSSARChecking(cluster2) + // wait for 2 seconds + time.Sleep(time.Second * 2) + results <- cluster2 + }() + }() + + // wait for c1 and c2 done with their work + for locker.inSSARChecking(cluster1) || locker.inSSARChecking(cluster2) { + time.Sleep(time.Second) + } + + // c1 works for 5 seconds + // c2 works for 2 seconds + // If c1 would hang c2, the c2 must return later, the results should be [cluster1, cluster2] + // Otherwise, the results should be [cluster2, cluster1].(And this result is what we expect) + r1 := <-results + r2 := <-results + if r1 != cluster2 || r2 != cluster1 { + t.Errorf("results not as expected, [%s, %s]", r1, r2) + } + +} diff --git a/pkg/operators/klusterlet/controllers/statuscontroller/klusterlet_status_controller.go b/pkg/operators/klusterlet/controllers/statuscontroller/klusterlet_status_controller.go index 94642f43e..29f55dac5 100644 --- a/pkg/operators/klusterlet/controllers/statuscontroller/klusterlet_status_controller.go +++ b/pkg/operators/klusterlet/controllers/statuscontroller/klusterlet_status_controller.go @@ -5,17 +5,11 @@ import ( "fmt" "strings" - authorizationv1 "k8s.io/api/authorization/v1" - corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/errors" - "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/runtime" appsinformer "k8s.io/client-go/informers/apps/v1" - coreinformer "k8s.io/client-go/informers/core/v1" "k8s.io/client-go/kubernetes" appslister "k8s.io/client-go/listers/apps/v1" - corelister "k8s.io/client-go/listers/core/v1" "k8s.io/klog/v2" "github.com/openshift/library-go/pkg/controller/factory" @@ -29,19 +23,16 @@ import ( type klusterletStatusController struct { kubeClient kubernetes.Interface - secretLister corelister.SecretLister deploymentLister appslister.DeploymentLister klusterletClient operatorv1client.KlusterletInterface klusterletLister operatorlister.KlusterletLister } const ( - klusterletNamespace = "open-cluster-management-agent" - klusterletRegistration = "Registration" - klusterletWork = "Work" - klusterletRegistrationDegraded = "KlusterletRegistrationDegraded" - klusterletWorKDegraded = "KlusterletWorkDegraded" - klusterletAvailable = "Available" + klusterletNamespace = "open-cluster-management-agent" + klusterletRegistrationDesiredDegraded = "RegistrationDesiredDegraded" + klusterletWorkDesiredDegraded = "WorkDesiredDegraded" + klusterletAvailable = "Available" ) // NewKlusterletStatusController returns a klusterletStatusController @@ -49,23 +40,16 @@ func NewKlusterletStatusController( kubeClient kubernetes.Interface, klusterletClient operatorv1client.KlusterletInterface, klusterletInformer operatorinformer.KlusterletInformer, - secretInformer coreinformer.SecretInformer, deploymentInformer appsinformer.DeploymentInformer, recorder events.Recorder) factory.Controller { controller := &klusterletStatusController{ kubeClient: kubeClient, klusterletClient: klusterletClient, - secretLister: secretInformer.Lister(), deploymentLister: deploymentInformer.Lister(), klusterletLister: klusterletInformer.Lister(), } return factory.New().WithSync(controller.sync). - WithInformersQueueKeyFunc(helpers.KlusterletSecretQueueKeyFunc(controller.klusterletLister), secretInformer.Informer()). WithInformersQueueKeyFunc(helpers.KlusterletDeploymentQueueKeyFunc(controller.klusterletLister), deploymentInformer.Informer()). - WithInformersQueueKeyFunc(func(obj runtime.Object) string { - accessor, _ := meta.Accessor(obj) - return accessor.GetName() - }, klusterletInformer.Informer()). ToController("KlusterletStatusController", recorder) } @@ -90,64 +74,69 @@ func (k *klusterletStatusController) sync(ctx context.Context, controllerContext klusterletNS = klusterletNamespace } - registrationDegradedCondition := checkAgentDegradedCondition( - ctx, k.kubeClient, - klusterletRegistration, klusterletRegistrationDegraded, - klusterletAgent{ - clusterName: klusterlet.Spec.ClusterName, - deploymentName: fmt.Sprintf("%s-registration-agent", klusterlet.Name), - namespace: klusterletNS, - getSSARFunc: getRegistrationSelfSubjectAccessReviews, - }, - []degradedCheckFunc{checkBootstrapSecret, checkHubConfigSecret, checkAgentDeployment}, - ) - workDegradedCondition := checkAgentDegradedCondition( - ctx, k.kubeClient, - klusterletWork, klusterletWorKDegraded, - klusterletAgent{ - clusterName: klusterlet.Spec.ClusterName, - deploymentName: fmt.Sprintf("%s-work-agent", klusterlet.Name), - namespace: klusterletNS, - getSSARFunc: getWorkSelfSubjectAccessReviews, - }, - []degradedCheckFunc{checkHubConfigSecret, checkAgentDeployment}, - ) + registrationDeploymentName := fmt.Sprintf("%s-registration-agent", klusterlet.Name) + workDeploymentName := fmt.Sprintf("%s-work-agent", klusterlet.Name) - availableCondition := checkAgentsDeployment( + availableCondition := checkAgentsDeploymentAvailable( ctx, k.kubeClient, []klusterletAgent{ { - clusterName: klusterlet.Spec.ClusterName, - deploymentName: fmt.Sprintf("%s-registration-agent", klusterlet.Name), + deploymentName: registrationDeploymentName, namespace: klusterletNS, - getSSARFunc: getWorkSelfSubjectAccessReviews, }, { - clusterName: klusterlet.Spec.ClusterName, - deploymentName: fmt.Sprintf("%s-work-agent", klusterlet.Name), + deploymentName: workDeploymentName, namespace: klusterletNS, - getSSARFunc: getWorkSelfSubjectAccessReviews, }, }, ) + registrationDesiredCondition := checkAgentDeploymentDired(ctx, k.kubeClient, klusterletNS, registrationDeploymentName, klusterletRegistrationDesiredDegraded) + workDesiredCondition := checkAgentDeploymentDired(ctx, k.kubeClient, klusterletNS, workDeploymentName, klusterletWorkDesiredDegraded) + _, _, err = helpers.UpdateKlusterletStatus(ctx, k.klusterletClient, klusterletName, - helpers.UpdateKlusterletConditionFn(registrationDegradedCondition), - helpers.UpdateKlusterletConditionFn(workDegradedCondition), helpers.UpdateKlusterletConditionFn(availableCondition), + helpers.UpdateKlusterletConditionFn(registrationDesiredCondition), + helpers.UpdateKlusterletConditionFn(workDesiredCondition), ) return err } type klusterletAgent struct { - clusterName string deploymentName string namespace string - getSSARFunc getSelfSubjectAccessReviewsFunc +} + +// Check agent deployment, if the desired replicas is not equal to available replicas, return degraded condition +func checkAgentDeploymentDired(ctx context.Context, kubeClient kubernetes.Interface, namespace, deploymentName, conditionType string) metav1.Condition { + deployment, err := kubeClient.AppsV1().Deployments(namespace).Get(ctx, deploymentName, metav1.GetOptions{}) + if err != nil { + return metav1.Condition{ + Type: conditionType, + Status: metav1.ConditionTrue, + Reason: "GetDeploymentFailed", + Message: fmt.Sprintf("Failed to get deployment %q %q: %v", namespace, deploymentName, err), + } + } + if unavailablePod := helpers.NumOfUnavailablePod(deployment); unavailablePod > 0 { + return metav1.Condition{ + Type: conditionType, + Status: metav1.ConditionTrue, + Reason: "UnavailablePods", + Message: fmt.Sprintf("%v of requested instances are unavailable of deployment %q %q", + unavailablePod, namespace, deploymentName), + } + } + return metav1.Condition{ + Type: conditionType, + Status: metav1.ConditionFalse, + Reason: "DeploymentsFunctional", + Message: fmt.Sprintf("deployments replicas are desired: %d", &deployment.Spec.Replicas), + } } // Check agent deployments, if both of them have at least 1 available replicas, return available condition -func checkAgentsDeployment(ctx context.Context, kubeClient kubernetes.Interface, agents []klusterletAgent) metav1.Condition { +func checkAgentsDeploymentAvailable(ctx context.Context, kubeClient kubernetes.Interface, agents []klusterletAgent) metav1.Condition { availableMessages := []string{} for _, agent := range agents { deployment, err := kubeClient.AppsV1().Deployments(agent.namespace).Get(ctx, agent.deploymentName, metav1.GetOptions{}) @@ -178,293 +167,3 @@ func checkAgentsDeployment(ctx context.Context, kubeClient kubernetes.Interface, Message: fmt.Sprintf("deployments are ready: %s", strings.Join(availableMessages, ",")), } } - -func checkAgentDegradedCondition( - ctx context.Context, kubeClient kubernetes.Interface, - agentName, degradedType string, - agent klusterletAgent, - degradedCheckFns []degradedCheckFunc) metav1.Condition { - degradedConditionReasons := []string{} - degradedConditionMessages := []string{} - for _, degradedCheckFn := range degradedCheckFns { - currCond := degradedCheckFn(ctx, kubeClient, agent) - if currCond == nil { - continue - } - degradedConditionReasons = append(degradedConditionReasons, currCond.Reason) - degradedConditionMessages = append(degradedConditionMessages, currCond.Message) - } - - if len(degradedConditionReasons) == 0 { - return metav1.Condition{ - Type: degradedType, - Status: metav1.ConditionFalse, - Reason: fmt.Sprintf("%sFunctional", agentName), - Message: fmt.Sprintf("%s is functioning correctly", agentName), - } - } - - return metav1.Condition{ - Type: degradedType, - Status: metav1.ConditionTrue, - Reason: strings.Join(degradedConditionReasons, ","), - Message: strings.Join(degradedConditionMessages, "\n"), - } -} - -type degradedCheckFunc func(ctx context.Context, kubeClient kubernetes.Interface, agent klusterletAgent) *metav1.Condition - -// Check bootstrap secret, if the secret is invalid, return registration degraded condition -func checkBootstrapSecret(ctx context.Context, kubeClient kubernetes.Interface, agent klusterletAgent) *metav1.Condition { - // Check if bootstrap secret exists - bootstrapSecret, err := kubeClient.CoreV1().Secrets(agent.namespace).Get(ctx, helpers.BootstrapHubKubeConfig, metav1.GetOptions{}) - if err != nil { - return &metav1.Condition{ - Reason: "BootstrapSecretMissing", - Message: fmt.Sprintf("Failed to get bootstrap secret %q %q: %v", agent.namespace, helpers.BootstrapHubKubeConfig, err), - } - } - - // Check if bootstrap secret works by building kube client - bootstrapClient, err := buildKubeClientWithSecret(bootstrapSecret) - if err != nil { - return &metav1.Condition{ - Reason: "BootstrapSecretError", - Message: fmt.Sprintf("Failed to build bootstrap kube client with bootstrap secret %q %q: %v", - agent.namespace, helpers.BootstrapHubKubeConfig, err), - } - } - - // Check the bootstrap client permissions by creating SelfSubjectAccessReviews - allowed, failedReview, err := createSelfSubjectAccessReviews(ctx, bootstrapClient, getBootstrapSelfSubjectAccessReviews()) - if err != nil { - return &metav1.Condition{ - Reason: "BootstrapSecretError", - Message: fmt.Sprintf("Failed to create %+v with bootstrap secret %q %q: %v", - failedReview, agent.namespace, helpers.BootstrapHubKubeConfig, err), - } - } - if !allowed { - return &metav1.Condition{ - Reason: "BootstrapSecretUnauthorized", - Message: fmt.Sprintf("Operation for resource %+v is not allowed with bootstrap secret %q %q", - failedReview.Spec.ResourceAttributes, agent.namespace, helpers.BootstrapHubKubeConfig), - } - } - - return nil -} - -// Check hub-kubeconfig-secret, if the secret is invalid, return degraded condition -func checkHubConfigSecret(ctx context.Context, kubeClient kubernetes.Interface, agent klusterletAgent) *metav1.Condition { - hubConfigSecret, err := kubeClient.CoreV1().Secrets(agent.namespace).Get(ctx, helpers.HubKubeConfig, metav1.GetOptions{}) - if err != nil { - return &metav1.Condition{ - Reason: "HubKubeConfigSecretMissing", - Message: fmt.Sprintf("Failed to get hub kubeconfig secret %q %q: %v", agent.namespace, helpers.HubKubeConfig, err), - } - } - - if hubConfigSecret.Data["kubeconfig"] == nil { - return &metav1.Condition{ - Reason: "HubKubeConfigMissing", - Message: fmt.Sprintf("Failed to get kubeconfig from `kubectl get secret -n %q %q -ojsonpath='{.data.kubeconfig}'`. "+ - "This is set by the klusterlet registration deployment, but the CSR must be approved by the cluster-admin on the hub.", - hubConfigSecret.Namespace, hubConfigSecret.Name), - } - } - - hubClient, err := buildKubeClientWithSecret(hubConfigSecret) - if err != nil { - return &metav1.Condition{ - Reason: "HubKubeConfigError", - Message: fmt.Sprintf("Failed to build hub kube client with hub config secret %q %q: %v", - hubConfigSecret.Namespace, hubConfigSecret.Name, err), - } - } - - clusterName := agent.clusterName - // If cluster name is empty, read cluster name from hub config secret - if clusterName == "" { - if hubConfigSecret.Data["cluster-name"] == nil { - return &metav1.Condition{ - Reason: "ClusterNameMissing", - Message: fmt.Sprintf( - "Failed to get cluster name from `kubectl get secret -n %q %q -ojsonpath='{.data.cluster-name}`."+ - " This is set by the klusterlet registration deployment.", hubConfigSecret.Namespace, hubConfigSecret.Name), - } - } - clusterName = string(hubConfigSecret.Data["cluster-name"]) - } - - // Check the hub kubeconfig permissions by creating SelfSubjectAccessReviews - allowed, failedReview, err := createSelfSubjectAccessReviews(ctx, hubClient, agent.getSSARFunc(agent.clusterName)) - if err != nil { - return &metav1.Condition{ - Reason: "HubKubeConfigError", - Message: fmt.Sprintf("Failed to create %+v with hub config secret %q %q: %v", - failedReview, hubConfigSecret.Namespace, hubConfigSecret.Name, err), - } - } - if !allowed { - return &metav1.Condition{ - Reason: "HubKubeConfigUnauthorized", - Message: fmt.Sprintf("Operation for resource %+v is not allowed with hub config secret %q %q", - failedReview.Spec.ResourceAttributes, hubConfigSecret.Namespace, hubConfigSecret.Name), - } - } - - return nil -} - -// Check agent deployment, if the desired replicas is not equal to available replicas, return degraded condition -func checkAgentDeployment(ctx context.Context, kubeClient kubernetes.Interface, agent klusterletAgent) *metav1.Condition { - deployment, err := kubeClient.AppsV1().Deployments(agent.namespace).Get(ctx, agent.deploymentName, metav1.GetOptions{}) - if err != nil { - return &metav1.Condition{ - Reason: "GetDeploymentFailed", - Message: fmt.Sprintf("Failed to get deployment %q %q: %v", agent.namespace, agent.deploymentName, err), - } - } - if unavailablePod := helpers.NumOfUnavailablePod(deployment); unavailablePod > 0 { - return &metav1.Condition{ - Reason: "UnavailablePods", - Message: fmt.Sprintf("%v of requested instances are unavailable of deployment %q %q", - unavailablePod, agent.namespace, agent.deploymentName), - } - } - return nil -} - -func buildKubeClientWithSecret(secret *corev1.Secret) (kubernetes.Interface, error) { - restConfig, err := helpers.LoadClientConfigFromSecret(secret) - if err != nil { - return nil, err - } - - return kubernetes.NewForConfig(restConfig) -} - -func createSelfSubjectAccessReviews( - ctx context.Context, - kubeClient kubernetes.Interface, - selfSubjectAccessReviews []authorizationv1.SelfSubjectAccessReview) (bool, *authorizationv1.SelfSubjectAccessReview, error) { - for i := range selfSubjectAccessReviews { - subjectAccessReview := selfSubjectAccessReviews[i] - ssar, err := kubeClient.AuthorizationV1().SelfSubjectAccessReviews().Create(ctx, &subjectAccessReview, metav1.CreateOptions{}) - if err != nil { - return false, &subjectAccessReview, err - } - if !ssar.Status.Allowed { - return false, &subjectAccessReview, nil - } - } - return true, nil, nil -} - -func getBootstrapSelfSubjectAccessReviews() []authorizationv1.SelfSubjectAccessReview { - reviews := []authorizationv1.SelfSubjectAccessReview{} - clusterResource := authorizationv1.ResourceAttributes{ - Group: "cluster.open-cluster-management.io", - Resource: "managedclusters", - } - reviews = append(reviews, generateSelfSubjectAccessReviews(clusterResource, "create", "get")...) - - certResource := authorizationv1.ResourceAttributes{ - Group: "certificates.k8s.io", - Resource: "certificatesigningrequests", - } - return append(reviews, generateSelfSubjectAccessReviews(certResource, "create", "get", "list", "watch")...) -} - -type getSelfSubjectAccessReviewsFunc func(string) []authorizationv1.SelfSubjectAccessReview - -func getRegistrationSelfSubjectAccessReviews(clusterName string) []authorizationv1.SelfSubjectAccessReview { - reviews := []authorizationv1.SelfSubjectAccessReview{} - certResource := authorizationv1.ResourceAttributes{ - Group: "certificates.k8s.io", - Resource: "certificatesigningrequests", - } - reviews = append(reviews, generateSelfSubjectAccessReviews(certResource, "get", "list", "watch")...) - - clusterResource := authorizationv1.ResourceAttributes{ - Group: "cluster.open-cluster-management.io", - Resource: "managedclusters", - Name: clusterName, - } - reviews = append(reviews, generateSelfSubjectAccessReviews(clusterResource, "get", "list", "update", "watch")...) - - clusterStatusResource := authorizationv1.ResourceAttributes{ - Group: "cluster.open-cluster-management.io", - Resource: "managedclusters", - Subresource: "status", - Name: clusterName, - } - reviews = append(reviews, generateSelfSubjectAccessReviews(clusterStatusResource, "patch", "update")...) - - clusterCertResource := authorizationv1.ResourceAttributes{ - Group: "register.open-cluster-management.io", - Resource: "managedclusters", - Subresource: "clientcertificates", - } - reviews = append(reviews, generateSelfSubjectAccessReviews(clusterCertResource, "renew")...) - - leaseResource := authorizationv1.ResourceAttributes{ - Group: "coordination.k8s.io", - Resource: "leases", - Name: fmt.Sprintf("cluster-lease-%s", clusterName), - Namespace: clusterName, - } - return append(reviews, generateSelfSubjectAccessReviews(leaseResource, "get", "update")...) -} - -func getWorkSelfSubjectAccessReviews(clusterName string) []authorizationv1.SelfSubjectAccessReview { - reviews := []authorizationv1.SelfSubjectAccessReview{} - eventResource := authorizationv1.ResourceAttributes{ - Resource: "events", - Namespace: clusterName, - } - reviews = append(reviews, generateSelfSubjectAccessReviews(eventResource, "create", "patch", "update")...) - - eventResource = authorizationv1.ResourceAttributes{ - Group: "events.k8s.io", - Resource: "events", - Namespace: clusterName, - } - reviews = append(reviews, generateSelfSubjectAccessReviews(eventResource, "create", "patch", "update")...) - - workResource := authorizationv1.ResourceAttributes{ - Group: "work.open-cluster-management.io", - Resource: "manifestworks", - Namespace: clusterName, - } - reviews = append(reviews, generateSelfSubjectAccessReviews(workResource, "get", "list", "watch", "update")...) - - workStatusResource := authorizationv1.ResourceAttributes{ - Group: "work.open-cluster-management.io", - Resource: "manifestworks", - Subresource: "status", - Namespace: clusterName, - } - reviews = append(reviews, generateSelfSubjectAccessReviews(workStatusResource, "patch", "update")...) - return reviews -} - -func generateSelfSubjectAccessReviews(resource authorizationv1.ResourceAttributes, verbs ...string) []authorizationv1.SelfSubjectAccessReview { - reviews := []authorizationv1.SelfSubjectAccessReview{} - for _, verb := range verbs { - reviews = append(reviews, authorizationv1.SelfSubjectAccessReview{ - Spec: authorizationv1.SelfSubjectAccessReviewSpec{ - ResourceAttributes: &authorizationv1.ResourceAttributes{ - Group: resource.Group, - Resource: resource.Resource, - Subresource: resource.Subresource, - Name: resource.Name, - Namespace: resource.Namespace, - Verb: verb, - }, - }, - }) - } - return reviews -} diff --git a/pkg/operators/klusterlet/controllers/statuscontroller/klusterlet_status_controller_test.go b/pkg/operators/klusterlet/controllers/statuscontroller/klusterlet_status_controller_test.go index 900b38d95..f93c1468a 100644 --- a/pkg/operators/klusterlet/controllers/statuscontroller/klusterlet_status_controller_test.go +++ b/pkg/operators/klusterlet/controllers/statuscontroller/klusterlet_status_controller_test.go @@ -2,15 +2,10 @@ package statuscontroller import ( "context" - "encoding/json" - "io/ioutil" - "net/http" - "net/http/httptest" "testing" "time" appsv1 "k8s.io/api/apps/v1" - authorizationv1 "k8s.io/api/authorization/v1" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" @@ -23,7 +18,6 @@ import ( fakeoperatorclient "open-cluster-management.io/api/client/operator/clientset/versioned/fake" operatorinformers "open-cluster-management.io/api/client/operator/informers/externalversions" operatorapiv1 "open-cluster-management.io/api/operator/v1" - "open-cluster-management.io/registration-operator/pkg/helpers" testinghelper "open-cluster-management.io/registration-operator/pkg/helpers/testing" ) @@ -107,7 +101,6 @@ func newTestController(klusterlet *operatorapiv1.Klusterlet, objects ...runtime. klusterletController := &klusterletStatusController{ kubeClient: fakeKubeClient, klusterletClient: fakeOperatorClient.OperatorV1().Klusterlets(), - secretLister: kubeInformers.Core().V1().Secrets().Lister(), deploymentLister: kubeInformers.Apps().V1().Deployments().Lister(), klusterletLister: operatorInformers.Operator().V1().Klusterlets().Lister(), } @@ -122,188 +115,77 @@ func newTestController(klusterlet *operatorapiv1.Klusterlet, objects ...runtime. } func TestSync(t *testing.T) { - response := &serverResponse{} - apiServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) { - if req.URL.Path != "/apis/authorization.k8s.io/v1/selfsubjectaccessreviews" { - w.WriteHeader(http.StatusNotFound) - return - } - - data, err := ioutil.ReadAll(req.Body) - if err != nil { - w.WriteHeader(http.StatusInternalServerError) - return - } - ssar := &authorizationv1.SelfSubjectAccessReview{} - json.Unmarshal(data, ssar) - if ssar.Spec.ResourceAttributes.Resource == "managedclusters" { - if ssar.Spec.ResourceAttributes.Subresource == "status" { - ssar.Status.Allowed = response.allowToOperateManagedClusterStatus - } else { - ssar.Status.Allowed = response.allowToOperateManagedClusters - } - } else if ssar.Spec.ResourceAttributes.Resource == "manifestworks" { - ssar.Status.Allowed = response.allowToOperateManifestWorks - } else { - ssar.Status.Allowed = true - } - - w.Header().Set("Content-type", "application/json") - w.WriteHeader(http.StatusCreated) - json.NewEncoder(w).Encode(ssar) - })) - defer apiServer.Close() - - apiServerHost := apiServer.URL - cases := []struct { - name string - object []runtime.Object - klusterlet *operatorapiv1.Klusterlet - allowToOperateManagedClusters bool - allowToOperateManagedClusterStatus bool - allowToOperateManifestWorks bool - expectedConditions []metav1.Condition + name string + object []runtime.Object + klusterlet *operatorapiv1.Klusterlet + + expectedConditions []metav1.Condition }{ { - name: "No bootstrap secret", - object: []runtime.Object{newSecret(helpers.HubKubeConfig, "test")}, - klusterlet: newKlusterlet("testklusterlet", "test", ""), - expectedConditions: []metav1.Condition{ - testinghelper.NamedCondition(klusterletRegistrationDegraded, "BootstrapSecretMissing,HubKubeConfigMissing,GetDeploymentFailed", metav1.ConditionTrue), - testinghelper.NamedCondition(klusterletWorKDegraded, "HubKubeConfigMissing,GetDeploymentFailed", metav1.ConditionTrue), - testinghelper.NamedCondition(klusterletAvailable, "GetDeploymentFailed", metav1.ConditionFalse), - }, - }, - { - name: "Bad bootstrap secret", + name: "Unavailable & Undesired", object: []runtime.Object{ - newSecret(helpers.HubKubeConfig, "test"), - newSecretWithKubeConfig(helpers.BootstrapHubKubeConfig, "test", []byte("badsecret")), - }, - klusterlet: newKlusterlet("testklusterlet", "test", ""), - expectedConditions: []metav1.Condition{ - testinghelper.NamedCondition(klusterletRegistrationDegraded, "BootstrapSecretError,HubKubeConfigMissing,GetDeploymentFailed", metav1.ConditionTrue), - testinghelper.NamedCondition(klusterletWorKDegraded, "HubKubeConfigMissing,GetDeploymentFailed", metav1.ConditionTrue), - testinghelper.NamedCondition(klusterletAvailable, "GetDeploymentFailed", metav1.ConditionFalse), - }, - }, - { - name: "Unauthorized bootstrap secret", - object: []runtime.Object{ - newSecret(helpers.HubKubeConfig, "test"), - newSecretWithKubeConfig(helpers.BootstrapHubKubeConfig, "test", newKubeConfig(apiServerHost)), - }, - klusterlet: newKlusterlet("testklusterlet", "test", ""), - expectedConditions: []metav1.Condition{ - testinghelper.NamedCondition(klusterletRegistrationDegraded, "BootstrapSecretUnauthorized,HubKubeConfigMissing,GetDeploymentFailed", metav1.ConditionTrue), - testinghelper.NamedCondition(klusterletWorKDegraded, "HubKubeConfigMissing,GetDeploymentFailed", metav1.ConditionTrue), - testinghelper.NamedCondition(klusterletAvailable, "GetDeploymentFailed", metav1.ConditionFalse), - }, - }, - { - name: "No hubconfig secret", - object: []runtime.Object{ - newSecretWithKubeConfig(helpers.BootstrapHubKubeConfig, "test", newKubeConfig(apiServerHost)), - }, - klusterlet: newKlusterlet("testklusterlet", "test", ""), - allowToOperateManagedClusters: true, - expectedConditions: []metav1.Condition{ - testinghelper.NamedCondition(klusterletRegistrationDegraded, "HubKubeConfigSecretMissing,GetDeploymentFailed", metav1.ConditionTrue), - testinghelper.NamedCondition(klusterletWorKDegraded, "HubKubeConfigSecretMissing,GetDeploymentFailed", metav1.ConditionTrue), - testinghelper.NamedCondition(klusterletAvailable, "GetDeploymentFailed", metav1.ConditionFalse), - }, - }, - { - name: "No cluster name secret", - object: []runtime.Object{ - newSecretWithKubeConfig(helpers.BootstrapHubKubeConfig, "test", newKubeConfig(apiServerHost)), - newSecretWithKubeConfig(helpers.HubKubeConfig, "test", newKubeConfig(apiServerHost)), - }, - allowToOperateManagedClusters: true, - klusterlet: newKlusterlet("testklusterlet", "test", ""), - expectedConditions: []metav1.Condition{ - testinghelper.NamedCondition(klusterletRegistrationDegraded, "ClusterNameMissing,GetDeploymentFailed", metav1.ConditionTrue), - testinghelper.NamedCondition(klusterletWorKDegraded, "ClusterNameMissing,GetDeploymentFailed", metav1.ConditionTrue), - testinghelper.NamedCondition(klusterletAvailable, "GetDeploymentFailed", metav1.ConditionFalse), - }, - }, - { - name: "No kubeconfig secret", - object: []runtime.Object{ - newSecretWithKubeConfig(helpers.BootstrapHubKubeConfig, "test", newKubeConfig(apiServerHost)), - newSecret(helpers.HubKubeConfig, "test"), - }, - allowToOperateManagedClusters: true, - klusterlet: newKlusterlet("testklusterlet", "test", "cluster1"), - expectedConditions: []metav1.Condition{ - testinghelper.NamedCondition(klusterletRegistrationDegraded, "HubKubeConfigMissing,GetDeploymentFailed", metav1.ConditionTrue), - testinghelper.NamedCondition(klusterletWorKDegraded, "HubKubeConfigMissing,GetDeploymentFailed", metav1.ConditionTrue), - testinghelper.NamedCondition(klusterletAvailable, "GetDeploymentFailed", metav1.ConditionFalse), - }, - }, - { - name: "Bad hub config secret", - object: []runtime.Object{ - newSecretWithKubeConfig(helpers.BootstrapHubKubeConfig, "test", newKubeConfig(apiServerHost)), - newSecretWithKubeConfig(helpers.HubKubeConfig, "test", []byte("badkubeconfig")), - }, - allowToOperateManagedClusters: true, - klusterlet: newKlusterlet("testklusterlet", "test", "cluster1"), - expectedConditions: []metav1.Condition{ - testinghelper.NamedCondition(klusterletRegistrationDegraded, "HubKubeConfigError,GetDeploymentFailed", metav1.ConditionTrue), - testinghelper.NamedCondition(klusterletWorKDegraded, "HubKubeConfigError,GetDeploymentFailed", metav1.ConditionTrue), - testinghelper.NamedCondition(klusterletAvailable, "GetDeploymentFailed", metav1.ConditionFalse), - }, - }, - { - name: "Unauthorized hub config secret", - object: []runtime.Object{ - newSecretWithKubeConfig(helpers.BootstrapHubKubeConfig, "test", newKubeConfig(apiServerHost)), - newSecretWithKubeConfig(helpers.HubKubeConfig, "test", newKubeConfig(apiServerHost)), - }, - allowToOperateManagedClusters: true, - klusterlet: newKlusterlet("testklusterlet", "test", "cluster1"), - expectedConditions: []metav1.Condition{ - testinghelper.NamedCondition(klusterletRegistrationDegraded, "HubKubeConfigUnauthorized,GetDeploymentFailed", metav1.ConditionTrue), - testinghelper.NamedCondition(klusterletWorKDegraded, "HubKubeConfigUnauthorized,GetDeploymentFailed", metav1.ConditionTrue), - testinghelper.NamedCondition(klusterletAvailable, "GetDeploymentFailed", metav1.ConditionFalse), - }, - }, - { - name: "Unavailable pod in deployments", - object: []runtime.Object{ - newSecretWithKubeConfig(helpers.BootstrapHubKubeConfig, "test", newKubeConfig(apiServerHost)), - newSecretWithKubeConfig(helpers.HubKubeConfig, "test", newKubeConfig(apiServerHost)), newDeployment("testklusterlet-registration-agent", "test", 3, 0), newDeployment("testklusterlet-work-agent", "test", 3, 0), }, - allowToOperateManagedClusters: true, - allowToOperateManagedClusterStatus: true, - allowToOperateManifestWorks: true, - klusterlet: newKlusterlet("testklusterlet", "test", "cluster1"), + + klusterlet: newKlusterlet("testklusterlet", "test", "cluster1"), expectedConditions: []metav1.Condition{ - testinghelper.NamedCondition(klusterletRegistrationDegraded, "UnavailablePods", metav1.ConditionTrue), - testinghelper.NamedCondition(klusterletWorKDegraded, "UnavailablePods", metav1.ConditionTrue), testinghelper.NamedCondition(klusterletAvailable, "NoAvailablePods", metav1.ConditionFalse), + testinghelper.NamedCondition(klusterletRegistrationDesiredDegraded, "UnavailablePods", metav1.ConditionTrue), + testinghelper.NamedCondition(klusterletWorkDesiredDegraded, "UnavailablePods", metav1.ConditionTrue), }, }, { - name: "Operator functional", + name: "Unavailable(by registration) & Undesired", + object: []runtime.Object{ + newDeployment("testklusterlet-registration-agent", "test", 3, 0), + newDeployment("testklusterlet-work-agent", "test", 3, 1), + }, + klusterlet: newKlusterlet("testklusterlet", "test", "cluster1"), + expectedConditions: []metav1.Condition{ + testinghelper.NamedCondition(klusterletAvailable, "NoAvailablePods", metav1.ConditionFalse), + testinghelper.NamedCondition(klusterletRegistrationDesiredDegraded, "UnavailablePods", metav1.ConditionTrue), + testinghelper.NamedCondition(klusterletWorkDesiredDegraded, "UnavailablePods", metav1.ConditionTrue), + }, + }, + { + name: "Unavailable(by work) & Undesired", + object: []runtime.Object{ + newDeployment("testklusterlet-registration-agent", "test", 3, 1), + newDeployment("testklusterlet-work-agent", "test", 3, 0), + }, + klusterlet: newKlusterlet("testklusterlet", "test", "cluster1"), + expectedConditions: []metav1.Condition{ + testinghelper.NamedCondition(klusterletAvailable, "NoAvailablePods", metav1.ConditionFalse), + testinghelper.NamedCondition(klusterletRegistrationDesiredDegraded, "UnavailablePods", metav1.ConditionTrue), + testinghelper.NamedCondition(klusterletWorkDesiredDegraded, "UnavailablePods", metav1.ConditionTrue), + }, + }, + { + name: "Available & Undesired", + object: []runtime.Object{ + newDeployment("testklusterlet-registration-agent", "test", 3, 1), + newDeployment("testklusterlet-work-agent", "test", 3, 1), + }, + klusterlet: newKlusterlet("testklusterlet", "test", "cluster1"), + expectedConditions: []metav1.Condition{ + testinghelper.NamedCondition(klusterletAvailable, "klusterletAvailable", metav1.ConditionTrue), + testinghelper.NamedCondition(klusterletRegistrationDesiredDegraded, "UnavailablePods", metav1.ConditionTrue), + testinghelper.NamedCondition(klusterletWorkDesiredDegraded, "UnavailablePods", metav1.ConditionTrue), + }, + }, + { + name: "Available & Desired", object: []runtime.Object{ - newSecretWithKubeConfig(helpers.BootstrapHubKubeConfig, "test", newKubeConfig(apiServerHost)), - newSecretWithKubeConfig(helpers.HubKubeConfig, "test", newKubeConfig(apiServerHost)), newDeployment("testklusterlet-registration-agent", "test", 3, 3), newDeployment("testklusterlet-work-agent", "test", 3, 3), }, - allowToOperateManagedClusters: true, - allowToOperateManagedClusterStatus: true, - allowToOperateManifestWorks: true, - klusterlet: newKlusterlet("testklusterlet", "test", "cluster1"), + klusterlet: newKlusterlet("testklusterlet", "test", "cluster1"), expectedConditions: []metav1.Condition{ - testinghelper.NamedCondition(klusterletRegistrationDegraded, "RegistrationFunctional", metav1.ConditionFalse), - testinghelper.NamedCondition(klusterletWorKDegraded, "WorkFunctional", metav1.ConditionFalse), testinghelper.NamedCondition(klusterletAvailable, "klusterletAvailable", metav1.ConditionTrue), + testinghelper.NamedCondition(klusterletRegistrationDesiredDegraded, "DeploymentsFunctional", metav1.ConditionFalse), + testinghelper.NamedCondition(klusterletWorkDesiredDegraded, "DeploymentsFunctional", metav1.ConditionFalse), }, }, } @@ -313,10 +195,6 @@ func TestSync(t *testing.T) { controller := newTestController(c.klusterlet, c.object...) syncContext := testinghelper.NewFakeSyncContext(t, c.klusterlet.Name) - response.allowToOperateManagedClusters = c.allowToOperateManagedClusters - response.allowToOperateManagedClusterStatus = c.allowToOperateManagedClusterStatus - response.allowToOperateManifestWorks = c.allowToOperateManifestWorks - err := controller.controller.sync(context.TODO(), syncContext) if err != nil { t.Errorf("Expected no error when update status: %v", err) diff --git a/pkg/operators/manager.go b/pkg/operators/manager.go index 0ec27e6a1..cb3ec4cab 100644 --- a/pkg/operators/manager.go +++ b/pkg/operators/manager.go @@ -24,6 +24,7 @@ import ( clustermanagerstatuscontroller "open-cluster-management.io/registration-operator/pkg/operators/clustermanager/controllers/statuscontroller" "open-cluster-management.io/registration-operator/pkg/operators/klusterlet/controllers/bootstrapcontroller" "open-cluster-management.io/registration-operator/pkg/operators/klusterlet/controllers/klusterletcontroller" + "open-cluster-management.io/registration-operator/pkg/operators/klusterlet/controllers/ssarcontroller" "open-cluster-management.io/registration-operator/pkg/operators/klusterlet/controllers/statuscontroller" ) @@ -152,11 +153,18 @@ func RunKlusterletOperator(ctx context.Context, controllerContext *controllercmd operatorNamespace, controllerContext.EventRecorder) - statusController := statuscontroller.NewKlusterletStatusController( + ssarController := ssarcontroller.NewKlustrletSSARController( kubeClient, operatorClient.OperatorV1().Klusterlets(), operatorInformer.Operator().V1().Klusterlets(), kubeInformer.Core().V1().Secrets(), + controllerContext.EventRecorder, + ) + + statusController := statuscontroller.NewKlusterletStatusController( + kubeClient, + operatorClient.OperatorV1().Klusterlets(), + operatorInformer.Operator().V1().Klusterlets(), kubeInformer.Apps().V1().Deployments(), controllerContext.EventRecorder, ) @@ -172,6 +180,7 @@ func RunKlusterletOperator(ctx context.Context, controllerContext *controllercmd go kubeInformer.Start(ctx.Done()) go klusterletController.Run(ctx, 1) go statusController.Run(ctx, 1) + go ssarController.Run(ctx, 1) go bootstrapController.Run(ctx, 1) <-ctx.Done() diff --git a/test/integration/klusterlet_test.go b/test/integration/klusterlet_test.go index ec75abd23..36e0f1673 100644 --- a/test/integration/klusterlet_test.go +++ b/test/integration/klusterlet_test.go @@ -493,8 +493,10 @@ var _ = ginkgo.Describe("Klusterlet", func() { _, err := operatorClient.OperatorV1().Klusterlets().Create(context.Background(), klusterlet, metav1.CreateOptions{}) gomega.Expect(err).NotTo(gomega.HaveOccurred()) - util.AssertKlusterletCondition(klusterlet.Name, operatorClient, "KlusterletRegistrationDegraded", "BootstrapSecretMissing,HubKubeConfigMissing,UnavailablePods", metav1.ConditionTrue) - util.AssertKlusterletCondition(klusterlet.Name, operatorClient, "KlusterletWorkDegraded", "HubKubeConfigMissing,UnavailablePods", metav1.ConditionTrue) + util.AssertKlusterletCondition(klusterlet.Name, operatorClient, "BootstrapSecretDegraded", "BootstrapSecretMissing", metav1.ConditionTrue) + util.AssertKlusterletCondition(klusterlet.Name, operatorClient, "HubConfigSecretDegraded", "HubKubeConfigMissing", metav1.ConditionTrue) + util.AssertKlusterletCondition(klusterlet.Name, operatorClient, "RegistrationDesiredDegraded", "UnavailablePods", metav1.ConditionTrue) + util.AssertKlusterletCondition(klusterlet.Name, operatorClient, "WorkDesiredDegraded", "UnavailablePods", metav1.ConditionTrue) // Create a bootstrap secret and make sure the kubeconfig can work bootStrapSecret := &corev1.Secret{ @@ -509,8 +511,10 @@ var _ = ginkgo.Describe("Klusterlet", func() { _, err = kubeClient.CoreV1().Secrets(klusterletNamespace).Create(context.Background(), bootStrapSecret, metav1.CreateOptions{}) gomega.Expect(err).ToNot(gomega.HaveOccurred()) - util.AssertKlusterletCondition(klusterlet.Name, operatorClient, "KlusterletRegistrationDegraded", "HubKubeConfigMissing,UnavailablePods", metav1.ConditionTrue) - util.AssertKlusterletCondition(klusterlet.Name, operatorClient, "KlusterletWorkDegraded", "HubKubeConfigMissing,UnavailablePods", metav1.ConditionTrue) + util.AssertKlusterletCondition(klusterlet.Name, operatorClient, "BootstrapSecretDegraded", "BootstrapSecretFunctional", metav1.ConditionFalse) + util.AssertKlusterletCondition(klusterlet.Name, operatorClient, "HubConfigSecretDegraded", "HubKubeConfigMissing", metav1.ConditionTrue) + util.AssertKlusterletCondition(klusterlet.Name, operatorClient, "RegistrationDesiredDegraded", "UnavailablePods", metav1.ConditionTrue) + util.AssertKlusterletCondition(klusterlet.Name, operatorClient, "WorkDesiredDegraded", "UnavailablePods", metav1.ConditionTrue) hubSecret, err := kubeClient.CoreV1().Secrets(klusterletNamespace).Get(context.Background(), helpers.HubKubeConfig, metav1.GetOptions{}) gomega.Expect(err).ToNot(gomega.HaveOccurred()) @@ -521,8 +525,10 @@ var _ = ginkgo.Describe("Klusterlet", func() { _, err = kubeClient.CoreV1().Secrets(klusterletNamespace).Update(context.Background(), hubSecret, metav1.UpdateOptions{}) gomega.Expect(err).ToNot(gomega.HaveOccurred()) - util.AssertKlusterletCondition(klusterlet.Name, operatorClient, "KlusterletRegistrationDegraded", "UnavailablePods", metav1.ConditionTrue) - util.AssertKlusterletCondition(klusterlet.Name, operatorClient, "KlusterletWorkDegraded", "UnavailablePods", metav1.ConditionTrue) + util.AssertKlusterletCondition(klusterlet.Name, operatorClient, "BootstrapSecretDegraded", "BootstrapSecretFunctional", metav1.ConditionFalse) + util.AssertKlusterletCondition(klusterlet.Name, operatorClient, "HubConfigSecretDegraded", "HubConfigSecretFunctional", metav1.ConditionFalse) + util.AssertKlusterletCondition(klusterlet.Name, operatorClient, "RegistrationDesiredDegraded", "UnavailablePods", metav1.ConditionTrue) + util.AssertKlusterletCondition(klusterlet.Name, operatorClient, "WorkDesiredDegraded", "UnavailablePods", metav1.ConditionTrue) // Update replica of deployment registrationDeployment, err := kubeClient.AppsV1().Deployments(klusterletNamespace).Get(context.Background(), registrationDeploymentName, metav1.GetOptions{}) @@ -540,8 +546,10 @@ var _ = ginkgo.Describe("Klusterlet", func() { _, err = kubeClient.AppsV1().Deployments(klusterletNamespace).UpdateStatus(context.Background(), workDeployment, metav1.UpdateOptions{}) gomega.Expect(err).ToNot(gomega.HaveOccurred()) - util.AssertKlusterletCondition(klusterlet.Name, operatorClient, "KlusterletRegistrationDegraded", "RegistrationFunctional", metav1.ConditionFalse) - util.AssertKlusterletCondition(klusterlet.Name, operatorClient, "KlusterletWorkDegraded", "WorkFunctional", metav1.ConditionFalse) + util.AssertKlusterletCondition(klusterlet.Name, operatorClient, "BootstrapSecretDegraded", "BootstrapSecretFunctional", metav1.ConditionFalse) + util.AssertKlusterletCondition(klusterlet.Name, operatorClient, "HubConfigSecretDegraded", "HubConfigSecretFunctional", metav1.ConditionFalse) + util.AssertKlusterletCondition(klusterlet.Name, operatorClient, "RegistrationDesiredDegraded", "DeploymentsFunctional", metav1.ConditionFalse) + util.AssertKlusterletCondition(klusterlet.Name, operatorClient, "WorkDesiredDegraded", "DeploymentsFunctional", metav1.ConditionFalse) }) ginkgo.It("should have correct available conditions", func() { diff --git a/test/integration/util/assertion.go b/test/integration/util/assertion.go index e1ef030d3..b18464fb5 100644 --- a/test/integration/util/assertion.go +++ b/test/integration/util/assertion.go @@ -11,7 +11,7 @@ import ( ) const ( - eventuallyTimeout = 30 // seconds + eventuallyTimeout = 60 // seconds eventuallyInterval = 1 // seconds )