diff --git a/cmd/aro/operator.go b/cmd/aro/operator.go index 801288a1c62..556c80918f1 100644 --- a/cmd/aro/operator.go +++ b/cmd/aro/operator.go @@ -154,7 +154,7 @@ func operator(ctx context.Context, log *logrus.Entry) error { if err = (checker.NewReconciler( log.WithField("controller", controllers.CheckerControllerName), - maocli, arocli, role, isDevelopmentMode)).SetupWithManager(mgr); err != nil { + maocli, arocli, kubernetescli, role, isDevelopmentMode)).SetupWithManager(mgr); err != nil { return fmt.Errorf("unable to create controller InternetChecker: %v", err) } diff --git a/pkg/operator/apis/aro.openshift.io/v1alpha1/cluster_types.go b/pkg/operator/apis/aro.openshift.io/v1alpha1/cluster_types.go index 4782d0039c7..31bd905f3a3 100644 --- a/pkg/operator/apis/aro.openshift.io/v1alpha1/cluster_types.go +++ b/pkg/operator/apis/aro.openshift.io/v1alpha1/cluster_types.go @@ -13,17 +13,18 @@ const ( InternetReachableFromMaster status.ConditionType = "InternetReachableFromMaster" InternetReachableFromWorker status.ConditionType = "InternetReachableFromWorker" MachineValid status.ConditionType = "MachineValid" + ServicePrincipalValid status.ConditionType = "ServicePrincipalValid" ) // AllConditionTypes is a operator conditions currently in use, any condition not in this list is not // added to the operator.status.conditions list func AllConditionTypes() []status.ConditionType { - return []status.ConditionType{InternetReachableFromMaster, InternetReachableFromWorker, MachineValid} + return []status.ConditionType{InternetReachableFromMaster, InternetReachableFromWorker, MachineValid, ServicePrincipalValid} } // ClusterChecksTypes represents checks performed on the cluster to verify basic functionality func ClusterChecksTypes() []status.ConditionType { - return []status.ConditionType{InternetReachableFromMaster, InternetReachableFromWorker, MachineValid} + return []status.ConditionType{InternetReachableFromMaster, InternetReachableFromWorker, MachineValid, ServicePrincipalValid} } type GenevaLoggingSpec struct { diff --git a/pkg/operator/controllers/checker/checker_controller.go b/pkg/operator/controllers/checker/checker_controller.go index c94e81bb8d3..c3840f5b828 100644 --- a/pkg/operator/controllers/checker/checker_controller.go +++ b/pkg/operator/controllers/checker/checker_controller.go @@ -12,6 +12,7 @@ import ( "github.com/sirupsen/logrus" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" + "k8s.io/client-go/kubernetes" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/builder" "sigs.k8s.io/controller-runtime/pkg/handler" @@ -32,11 +33,14 @@ type CheckerController struct { checkers []Checker } -func NewReconciler(log *logrus.Entry, maocli maoclient.Interface, arocli aroclient.Interface, role string, isDevelopmentMode bool) *CheckerController { +func NewReconciler(log *logrus.Entry, maocli maoclient.Interface, arocli aroclient.Interface, kubernetescli kubernetes.Interface, role string, isDevelopmentMode bool) *CheckerController { checkers := []Checker{NewInternetChecker(log, arocli, role)} if role == operator.RoleMaster { - checkers = append(checkers, NewMachineChecker(log, maocli, arocli, role, isDevelopmentMode)) + checkers = append(checkers, + NewMachineChecker(log, maocli, arocli, role, isDevelopmentMode), + NewServicePrincipalChecker(log, maocli, arocli, kubernetescli, role), + ) } return &CheckerController{ diff --git a/pkg/operator/controllers/checker/const.go b/pkg/operator/controllers/checker/const.go new file mode 100644 index 00000000000..a4f70c30456 --- /dev/null +++ b/pkg/operator/controllers/checker/const.go @@ -0,0 +1,10 @@ +package checker + +// Copyright (c) Microsoft Corporation. +// Licensed under the Apache License 2.0. + +const ( + azureCredentialSecretNamespace = "kube-system" + azureCredentialSecretName = "azure-credentials" + machineSetsNamespace = "openshift-machine-api" +) diff --git a/pkg/operator/controllers/checker/machinechecker.go b/pkg/operator/controllers/checker/machinechecker.go index a28ba904f0f..08c5dc49990 100644 --- a/pkg/operator/controllers/checker/machinechecker.go +++ b/pkg/operator/controllers/checker/machinechecker.go @@ -25,10 +25,6 @@ import ( _ "github.com/Azure/ARO-RP/pkg/util/scheme" ) -const ( - machineSetsNamespace = "openshift-machine-api" -) - // MachineChecker reconciles the alertmanager webhook type MachineChecker struct { clustercli maoclient.Interface diff --git a/pkg/operator/controllers/checker/serviceprincipalchecker.go b/pkg/operator/controllers/checker/serviceprincipalchecker.go new file mode 100644 index 00000000000..3e2b1f680bc --- /dev/null +++ b/pkg/operator/controllers/checker/serviceprincipalchecker.go @@ -0,0 +1,121 @@ +package checker + +// Copyright (c) Microsoft Corporation. +// Licensed under the Apache License 2.0. + +import ( + "context" + + "github.com/Azure/go-autorest/autorest/azure" + maoclient "github.com/openshift/machine-api-operator/pkg/generated/clientset/versioned" + "github.com/operator-framework/operator-sdk/pkg/status" + "github.com/sirupsen/logrus" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/client-go/kubernetes" + + "github.com/Azure/ARO-RP/pkg/api" + "github.com/Azure/ARO-RP/pkg/api/validate/dynamic" + arov1alpha1 "github.com/Azure/ARO-RP/pkg/operator/apis/aro.openshift.io/v1alpha1" + aroclient "github.com/Azure/ARO-RP/pkg/operator/clientset/versioned" + "github.com/Azure/ARO-RP/pkg/operator/controllers" + "github.com/Azure/ARO-RP/pkg/util/aad" +) + +type ServicePrincipalChecker struct { + log *logrus.Entry + clustercli maoclient.Interface + arocli aroclient.Interface + kubernetescli kubernetes.Interface + role string +} + +func NewServicePrincipalChecker(log *logrus.Entry, maocli maoclient.Interface, arocli aroclient.Interface, kubernetescli kubernetes.Interface, role string) *ServicePrincipalChecker { + return &ServicePrincipalChecker{ + log: log, + clustercli: maocli, + arocli: arocli, + kubernetescli: kubernetescli, + role: role, + } +} + +func (r *ServicePrincipalChecker) Name() string { + return "ServicePrincipalChecker" +} + +func (r *ServicePrincipalChecker) Check(ctx context.Context) error { + cond := &status.Condition{ + Type: arov1alpha1.ServicePrincipalValid, + Status: corev1.ConditionTrue, + Message: "service principal is valid", + Reason: "CheckDone", + } + + cluster, err := r.arocli.AroV1alpha1().Clusters().Get(ctx, arov1alpha1.SingletonClusterName, metav1.GetOptions{}) + if err != nil { + return err + } + + resource, err := azure.ParseResourceID(cluster.Spec.ResourceID) + if err != nil { + return err + } + + azEnv, err := azure.EnvironmentFromName(cluster.Spec.AZEnvironment) + if err != nil { + return err + } + + azCred, err := azCredentials(ctx, r.kubernetescli) + if err != nil { + return err + } + + _, err = aad.GetToken(ctx, r.log, azCred.clientID, azCred.clientSecret, azCred.tenantID, azEnv.ActiveDirectoryEndpoint, azEnv.ResourceManagerEndpoint) + if err != nil { + updateFailedCondition(cond, err) + } + + spDynamic, err := dynamic.NewValidator(r.log, &azEnv, resource.SubscriptionID, nil, dynamic.AuthorizerClusterServicePrincipal) + if err != nil { + return err + } + + err = spDynamic.ValidateServicePrincipal(ctx, azCred.clientID, azCred.clientSecret, azCred.tenantID) + if err != nil { + updateFailedCondition(cond, err) + } + + return controllers.SetCondition(ctx, r.arocli, cond, r.role) +} + +type credentials struct { + clientID string + clientSecret string + tenantID string +} + +func azCredentials(ctx context.Context, kubernetescli kubernetes.Interface) (*credentials, error) { + var creds credentials + + mysec, err := kubernetescli.CoreV1().Secrets(azureCredentialSecretNamespace).Get(ctx, azureCredentialSecretName, metav1.GetOptions{}) + if err != nil { + return nil, err + } + + creds.clientID = string(mysec.Data["azure_client_id"]) + creds.clientSecret = string(mysec.Data["azure_client_secret"]) + creds.tenantID = string(mysec.Data["azure_tenant_id"]) + + return &creds, nil +} + +func updateFailedCondition(cond *status.Condition, err error) { + cond.Status = corev1.ConditionFalse + if tErr, ok := err.(*api.CloudError); ok { + cond.Message = tErr.Message + } else { + cond.Message = err.Error() + } +} diff --git a/pkg/operator/controllers/checker/serviceprincipalchecker_test.go b/pkg/operator/controllers/checker/serviceprincipalchecker_test.go new file mode 100644 index 00000000000..7bd13770217 --- /dev/null +++ b/pkg/operator/controllers/checker/serviceprincipalchecker_test.go @@ -0,0 +1,93 @@ +package checker + +// Copyright (c) Microsoft Corporation. +// Licensed under the Apache License 2.0. + +import ( + "context" + "fmt" + "testing" + + azuretypes "github.com/openshift/installer/pkg/types/azure" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/client-go/kubernetes/fake" + + arov1alpha1 "github.com/Azure/ARO-RP/pkg/operator/apis/aro.openshift.io/v1alpha1" + arofake "github.com/Azure/ARO-RP/pkg/operator/clientset/versioned/fake" +) + +// TODO - once aad.GetToken is mockable add tests for other cases +func TestServicePrincipalValid(t *testing.T) { + ctx := context.Background() + + for _, tt := range []struct { + name string + aroCluster *arov1alpha1.Cluster + wantErr string + }{ + { + name: "fail: aro cluster resource doesn't exist", + wantErr: `clusters.aro.openshift.io "cluster" not found`, + }, + { + name: "fail: invalid cluster resource", + aroCluster: &arov1alpha1.Cluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: arov1alpha1.SingletonClusterName, + }, + Spec: arov1alpha1.ClusterSpec{ + AZEnvironment: azuretypes.PublicCloud.Name(), + ResourceID: "invalid_resource", + }, + }, + wantErr: `parsing failed for invalid_resource. Invalid resource Id format`, + }, + { + name: "fail: invalid az environment", + aroCluster: &arov1alpha1.Cluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: arov1alpha1.SingletonClusterName, + }, + Spec: arov1alpha1.ClusterSpec{ + AZEnvironment: "NEVERLAND", + ResourceID: "/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/myresourcegroup/providers/Microsoft.RedHatOpenShift/openShiftClusters/mycluster", + }, + }, + wantErr: `autorest/azure: There is no cloud environment matching the name "NEVERLAND"`, + }, + { + name: "fail: azure-credential secret doesn't exist", + aroCluster: &arov1alpha1.Cluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: arov1alpha1.SingletonClusterName, + }, + Spec: arov1alpha1.ClusterSpec{ + AZEnvironment: azuretypes.PublicCloud.Name(), + ResourceID: "/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/myresourcegroup/providers/Microsoft.RedHatOpenShift/openShiftClusters/mycluster", + }, + }, + wantErr: `secrets "azure-credentials" not found`, + }, + } { + arocli := arofake.NewSimpleClientset() + kubernetescli := fake.NewSimpleClientset() + + if tt.aroCluster != nil { + arocli = arofake.NewSimpleClientset(tt.aroCluster) + } + + sp := &ServicePrincipalChecker{ + arocli: arocli, + kubernetescli: kubernetescli, + } + + t.Run(tt.name, func(t *testing.T) { + err := sp.Check(ctx) + + if err != nil && err.Error() != tt.wantErr || + err == nil && tt.wantErr != "" { + t.Error(fmt.Errorf("\n%s\n !=\n%s", err.Error(), tt.wantErr)) + } + }) + } +}