Expose DNS conditions on the ARO Operator

cmd/aro/operator.go

@@ -130,8 +130,8 @@ func operator(ctx context.Context, log *logrus.Entry) error {
 
 	if err = (checker.NewReconciler(
 		log.WithField("controller", controllers.CheckerControllerName),
-		maocli, arocli, role, deploymentMode)).SetupWithManager(mgr); err != nil {
-		return fmt.Errorf("unable to create controller InternetChecker: %v", err)
+		maocli, arocli, kubernetescli, role, deploymentMode)).SetupWithManager(mgr); err != nil {
+		return fmt.Errorf("unable to create controller Checker: %v", err)
 	}
 
 	// +kubebuilder:scaffold:builder

pkg/api/validate/dynamic.go

@@ -18,7 +18,6 @@ import (
 	"k8s.io/apimachinery/pkg/util/wait"
 
 	"github.com/Azure/ARO-RP/pkg/api"
-	"github.com/Azure/ARO-RP/pkg/env"
 	"github.com/Azure/ARO-RP/pkg/util/azureclient/mgmt/authorization"
 	"github.com/Azure/ARO-RP/pkg/util/azureclient/mgmt/network"
 	utilpermissions "github.com/Azure/ARO-RP/pkg/util/permissions"
@@ -29,27 +28,26 @@ import (
 
 // SlimDynamic validate in the operator context.
 type SlimDynamic interface {
-	ValidateVnetPermissions(ctx context.Context) error
-	ValidateRouteTablesPermissions(ctx context.Context) error
-	ValidateVnetDns(ctx context.Context) error
+	ValidateVnetPermissions(ctx context.Context, code string, typ string) error
+	ValidateRouteTablesPermissions(ctx context.Context, code string, typ string) error
+	ValidateVnetDNS(ctx context.Context) error
 	// etc
 	// does Quota code go in here too?
 }
 
+var _ SlimDynamic = (*dynamic)(nil)
+
 type dynamic struct {
 	log             *logrus.Entry
 	vnetr           *azure.Resource
 	masterSubnetID  string
 	workerSubnetIDs []string
 
-	code string
-	typ  string
-
 	permissions     authorization.PermissionsClient
 	virtualNetworks virtualNetworksGetClient
 }
 
-func NewValidator(log *logrus.Entry, env env.Interface, masterSubnetID string, workerSubnetIDs []string, subscriptionID string, authorizer refreshable.Authorizer, code string, typ string) (*dynamic, error) {
+func NewValidator(log *logrus.Entry, azEnv *azure.Environment, masterSubnetID string, workerSubnetIDs []string, subscriptionID string, authorizer refreshable.Authorizer) (*dynamic, error) {
 	vnetID, _, err := subnet.Split(masterSubnetID)
 	if err != nil {
 		return nil, err
@@ -66,16 +64,25 @@ func NewValidator(log *logrus.Entry, env env.Interface, masterSubnetID string, w
 		masterSubnetID:  masterSubnetID,
 		workerSubnetIDs: workerSubnetIDs,
 
-		code: code,
-		typ:  typ,
-
-		permissions:     authorization.NewPermissionsClient(env.Environment(), subscriptionID, authorizer),
-		virtualNetworks: newVirtualNetworksCache(network.NewVirtualNetworksClient(env.Environment(), subscriptionID, authorizer)),
+		permissions:     authorization.NewPermissionsClient(azEnv, subscriptionID, authorizer),
+		virtualNetworks: newVirtualNetworksCache(network.NewVirtualNetworksClient(azEnv, subscriptionID, authorizer)),
 	}, nil
 }
 
-func (dv *dynamic) ValidateVnetPermissions(ctx context.Context) error {
-	dv.log.Printf("ValidateVnetPermissions (%s)", dv.typ)
+func NewSlimDynamicValidator(log *logrus.Entry, azEnv *azure.Environment, vnetResource azure.Resource, masterSubnetID string, workerSubnetsIDs []string, subscriptionID string, authorizer refreshable.Authorizer) SlimDynamic {
+
+	return &dynamic{
+		log:             log,
+		vnetr:           &vnetResource,
+		masterSubnetID:  masterSubnetID,
+		workerSubnetIDs: workerSubnetsIDs,
+		permissions:     authorization.NewPermissionsClient(azEnv, subscriptionID, authorizer),
+		virtualNetworks: newVirtualNetworksCache(network.NewVirtualNetworksClient(azEnv, subscriptionID, authorizer)),
+	}
+
+}
+func (dv *dynamic) ValidateVnetPermissions(ctx context.Context, code string, typ string) error {
+	dv.log.Printf("ValidateVnetPermissions (%s)", typ)
 
 	err := dv.validateActions(ctx, dv.vnetr, []string{
 		"Microsoft.Network/virtualNetworks/join/action",
@@ -87,7 +94,7 @@ func (dv *dynamic) ValidateVnetPermissions(ctx context.Context) error {
 	})
 
 	if err == wait.ErrWaitTimeout {
-		return api.NewCloudError(http.StatusBadRequest, dv.code, "", "The %s does not have Network Contributor permission on vnet '%s'.", dv.typ, dv.vnetr)
+		return api.NewCloudError(http.StatusBadRequest, code, "", "The %s does not have Network Contributor permission on vnet '%s'.", typ, dv.vnetr)
 	}
 	if detailedErr, ok := err.(autorest.DetailedError); ok &&
 		detailedErr.StatusCode == http.StatusNotFound {
@@ -96,7 +103,7 @@ func (dv *dynamic) ValidateVnetPermissions(ctx context.Context) error {
 	return err
 }
 
-func (dv *dynamic) ValidateRouteTablesPermissions(ctx context.Context) error {
+func (dv *dynamic) ValidateRouteTablesPermissions(ctx context.Context, code string, typ string) error {
 	vnet, err := dv.virtualNetworks.Get(ctx, dv.vnetr.ResourceGroup, dv.vnetr.ResourceName, "")
 	if err != nil {
 		return err
@@ -135,7 +142,7 @@ func (dv *dynamic) ValidateRouteTablesPermissions(ctx context.Context) error {
 	sort.Slice(rts, func(i, j int) bool { return strings.Compare(m[rts[i]], m[rts[j]]) < 0 })
 
 	for _, rt := range rts {
-		err := dv.validateRouteTablePermissions(ctx, rt, m[rt])
+		err := dv.validateRouteTablePermissions(ctx, rt, m[rt], code, typ)
 		if err != nil {
 			return err
 		}
@@ -144,8 +151,8 @@ func (dv *dynamic) ValidateRouteTablesPermissions(ctx context.Context) error {
 	return nil
 }
 
-func (dv *dynamic) validateRouteTablePermissions(ctx context.Context, rtID string, path string) error {
-	dv.log.Printf("validateRouteTablePermissions(%s, %s)", dv.typ, path)
+func (dv *dynamic) validateRouteTablePermissions(ctx context.Context, rtID string, path string, code string, typ string) error {
+	dv.log.Printf("validateRouteTablePermissions(%s, %s)", typ, path)
 
 	rtr, err := azure.ParseResourceID(rtID)
 	if err != nil {
@@ -158,7 +165,7 @@ func (dv *dynamic) validateRouteTablePermissions(ctx context.Context, rtID strin
 		"Microsoft.Network/routeTables/write",
 	})
 	if err == wait.ErrWaitTimeout {
-		return api.NewCloudError(http.StatusBadRequest, dv.code, "", "The %s does not have Network Contributor permission on route table '%s'.", dv.typ, rtID)
+		return api.NewCloudError(http.StatusBadRequest, code, "", "The %s does not have Network Contributor permission on route table '%s'.", typ, rtID)
 	}
 	if detailedErr, ok := err.(autorest.DetailedError); ok &&
 		detailedErr.StatusCode == http.StatusNotFound {

pkg/api/validate/dynamic_test.go

@@ -18,6 +18,7 @@ import (
 	"github.com/golang/mock/gomock"
 	"github.com/sirupsen/logrus"
 
+	"github.com/Azure/ARO-RP/pkg/api"
 	mock_authorization "github.com/Azure/ARO-RP/pkg/util/mocks/azureclient/mgmt/authorization"
 	mock_network "github.com/Azure/ARO-RP/pkg/util/mocks/azureclient/mgmt/network"
 )
@@ -115,11 +116,9 @@ func TestValidateVnetPermissions(t *testing.T) {
 					ResourceName:   vnetName,
 					SubscriptionID: subscriptionID,
 				},
-				code: "InvalidResourceProviderPermissions",
-				typ:  "resource provider",
 			}
 
-			err := dv.ValidateVnetPermissions(ctx)
+			err := dv.ValidateVnetPermissions(ctx, api.CloudErrorCodeInvalidResourceProviderPermissions, "resource provider")
 			if err != nil && err.Error() != tt.wantErr ||
 				err == nil && tt.wantErr != "" {
 				t.Error(err)
@@ -347,12 +346,10 @@ func TestValidateRouteTablePermissions(t *testing.T) {
 		dv := &dynamic{
 			log:         logrus.NewEntry(logrus.StandardLogger()),
 			permissions: permissionsClient,
-			code:        "InvalidResourceProviderPermissions",
-			typ:         "resource provider",
 		}
 
 		// purposefully hardcoding path to "" so it is not needed in the wantErr message
-		err := dv.validateRouteTablePermissions(ctx, tt.rtID, "")
+		err := dv.validateRouteTablePermissions(ctx, tt.rtID, "", api.CloudErrorCodeInvalidResourceProviderPermissions, "resource provider")
 		if err != nil && err.Error() != tt.wantErr ||
 			err == nil && tt.wantErr != "" {
 			t.Error(err)
@@ -506,9 +503,6 @@ func TestValidateRouteTablesPermissions(t *testing.T) {
 
 				masterSubnetID:  masterSubnet,
 				workerSubnetIDs: []string{workerSubnet},
-
-				code: "InvalidResourceProviderPermissions",
-				typ:  "resource provider",
 			}
 
 			if tt.permissionMocks != nil {
@@ -519,7 +513,7 @@ func TestValidateRouteTablesPermissions(t *testing.T) {
 				tt.vnetMocks(vnetClient, *vnet)
 			}
 
-			err := dv.ValidateRouteTablesPermissions(ctx)
+			err := dv.ValidateRouteTablesPermissions(ctx, api.CloudErrorCodeInvalidResourceProviderPermissions, "resource provider")
 			if err != nil && err.Error() != tt.wantErr ||
 				err == nil && tt.wantErr != "" {
 				t.Error(err)

pkg/api/validate/openshiftcluster_validatedynamic.go

@@ -42,11 +42,11 @@ func NewOpenShiftClusterDynamicValidator(log *logrus.Entry, env env.Interface, o
 	}
 }
 
-type azureClaim struct {
+type AzureClaim struct {
 	Roles []string `json:"roles,omitempty"`
 }
 
-func (*azureClaim) Valid() error {
+func (*AzureClaim) Valid() error {
 	return fmt.Errorf("unimplemented")
 }
 
@@ -71,17 +71,17 @@ func (dv *openShiftClusterDynamicValidator) Dynamic(ctx context.Context) error {
 	}
 
 	// FP validation
-	fpDynamic, err := NewValidator(dv.log, dv.env, mSubnetID, wSubnetIDs, dv.subscriptionDoc.ID, dv.fpAuthorizer, api.CloudErrorCodeInvalidResourceProviderPermissions, "resource provider")
+	fpDynamic, err := NewValidator(dv.log, dv.env.Environment(), mSubnetID, wSubnetIDs, dv.subscriptionDoc.ID, dv.fpAuthorizer)
 	if err != nil {
 		return err
 	}
 
-	err = fpDynamic.ValidateVnetPermissions(ctx)
+	err = fpDynamic.ValidateVnetPermissions(ctx, api.CloudErrorCodeInvalidResourceProviderPermissions, "resource provider")
 	if err != nil {
 		return err
 	}
 
-	err = fpDynamic.ValidateRouteTablesPermissions(ctx)
+	err = fpDynamic.ValidateRouteTablesPermissions(ctx, api.CloudErrorCodeInvalidResourceProviderPermissions, "resource provider")
 	if err != nil {
 		return err
 	}
@@ -92,24 +92,24 @@ func (dv *openShiftClusterDynamicValidator) Dynamic(ctx context.Context) error {
 		return err
 	}
 
-	token, err := aad.GetToken(ctx, dv.log, dv.oc, dv.subscriptionDoc, dv.env.Environment().ActiveDirectoryEndpoint, dv.env.Environment().ResourceManagerEndpoint)
+	token, err := aad.GetToken(ctx, dv.log, dv.oc.Properties.ServicePrincipalProfile.ClientID, dv.oc.Properties.ServicePrincipalProfile.ClientSecret, dv.subscriptionDoc.Subscription.Properties.TenantID, dv.env.Environment().ActiveDirectoryEndpoint, dv.env.Environment().ResourceManagerEndpoint)
 	if err != nil {
 		return err
 	}
 
 	spAuthorizer := refreshable.NewAuthorizer(token)
 
-	spDynamic, err := NewValidator(dv.log, dv.env, mSubnetID, wSubnetIDs, dv.subscriptionDoc.ID, spAuthorizer, api.CloudErrorCodeInvalidServicePrincipalPermissions, "provided service principal")
+	spDynamic, err := NewValidator(dv.log, dv.env.Environment(), mSubnetID, wSubnetIDs, dv.subscriptionDoc.ID, spAuthorizer)
 	if err != nil {
 		return err
 	}
 
-	err = spDynamic.ValidateVnetPermissions(ctx)
+	err = spDynamic.ValidateVnetPermissions(ctx, api.CloudErrorCodeInvalidServicePrincipalPermissions, "provided service principal")
 	if err != nil {
 		return err
 	}
 
-	err = spDynamic.ValidateRouteTablesPermissions(ctx)
+	err = spDynamic.ValidateRouteTablesPermissions(ctx, api.CloudErrorCodeInvalidServicePrincipalPermissions, "provided service principal")
 	if err != nil {
 		return err
 	}
@@ -300,13 +300,13 @@ func validateServicePrincipalProfile(ctx context.Context, log *logrus.Entry, env
 
 	log.Print("validateServicePrincipalProfile")
 
-	token, err := aad.GetToken(ctx, log, oc, sub, env.Environment().ActiveDirectoryEndpoint, env.Environment().GraphEndpoint)
+	token, err := aad.GetToken(ctx, log, oc.Properties.ServicePrincipalProfile.ClientID, oc.Properties.ServicePrincipalProfile.ClientSecret, sub.Subscription.Properties.TenantID, env.Environment().ActiveDirectoryEndpoint, env.Environment().ResourceManagerEndpoint)
 	if err != nil {
 		return err
 	}
 
 	p := &jwt.Parser{}
-	c := &azureClaim{}
+	c := &AzureClaim{}
 	_, _, err = p.ParseUnverified(token.OAuthToken(), c)
 	if err != nil {
 		return err

pkg/api/validate/quota.go

@@ -108,7 +108,7 @@ func NewAzureQuotaValidator(ctx context.Context, log *logrus.Entry, env env.Inte
 		return nil, err
 	}
 
-	token, err := aad.GetToken(ctx, log, oc, subscriptionDoc, env.Environment().ActiveDirectoryEndpoint, env.Environment().ResourceManagerEndpoint)
+	token, err := aad.GetToken(ctx, log, oc.Properties.ServicePrincipalProfile.ClientID, oc.Properties.ServicePrincipalProfile.ClientSecret, subscriptionDoc.Subscription.Properties.TenantID, env.Environment().ActiveDirectoryEndpoint, env.Environment().ResourceManagerEndpoint)
 	if err != nil {
 		return nil, err
 	}

pkg/cluster/deploystorage.go

@@ -42,7 +42,7 @@ func (m *manager) clusterSPObjectID(ctx context.Context) (string, error) {
 	var clusterSPObjectID string
 	spp := &m.doc.OpenShiftCluster.Properties.ServicePrincipalProfile
 
-	token, err := aad.GetToken(ctx, m.log, m.doc.OpenShiftCluster, m.subscriptionDoc, m.env.Environment().ActiveDirectoryEndpoint, m.env.Environment().GraphEndpoint)
+	token, err := aad.GetToken(ctx, m.log, m.doc.OpenShiftCluster.Properties.ServicePrincipalProfile.ClientID, m.doc.OpenShiftCluster.Properties.ServicePrincipalProfile.ClientSecret, m.subscriptionDoc.Subscription.Properties.TenantID, m.env.Environment().ActiveDirectoryEndpoint, m.env.Environment().GraphEndpoint)
 	if err != nil {
 		return "", err
 	}

pkg/operator/apis/aro.openshift.io/v1alpha1/cluster_types.go

@@ -13,10 +13,11 @@ const (
 	InternetReachableFromMaster status.ConditionType = "InternetReachableFromMaster"
 	InternetReachableFromWorker status.ConditionType = "InternetReachableFromWorker"
 	MachineValid                status.ConditionType = "MachineValid"
+	DNSValid                    status.ConditionType = "DNSValid"
 )
 
 func AllConditionTypes() []status.ConditionType {
-	return []status.ConditionType{InternetReachableFromMaster, InternetReachableFromWorker, MachineValid}
+	return []status.ConditionType{InternetReachableFromMaster, InternetReachableFromWorker, MachineValid, DNSValid}
 }
 
 type GenevaLoggingSpec struct {

pkg/operator/controllers/checker/auth.go

@@ -0,0 +1,66 @@
+package checker
+
+// Copyright (c) Microsoft Corporation.
+// Licensed under the Apache License 2.0.
+
+import (
+	"context"
+	"errors"
+	"net/http"
+
+	"github.com/Azure/go-autorest/autorest/adal"
+	jwt "github.com/form3tech-oss/jwt-go"
+	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"
+	"github.com/Azure/ARO-RP/pkg/util/refreshable"
+)
+
+type credentials struct {
+	clientID     []byte
+	clientSecret []byte
+	tenantID     []byte
+}
+
+func newAuthorizer(token *adal.ServicePrincipalToken) (refreshable.Authorizer, error) {
+	p := &jwt.Parser{}
+	c := &validate.AzureClaim{}
+	_, _, err := p.ParseUnverified(token.OAuthToken(), c)
+	if err != nil {
+		return nil, err
+	}
+
+	for _, role := range c.Roles {
+		if role == "Application.ReadWrite.OwnedBy" {
+			return nil, api.NewCloudError(http.StatusBadRequest, api.CloudErrorCodeInvalidServicePrincipalCredentials, "properties.servicePrincipalProfile", "The provided service principal must not have the Application.ReadWrite.OwnedBy permission.")
+		}
+	}
+
+	return refreshable.NewAuthorizer(token), nil
+}
+
+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
+	}
+	if _, ok := mysec.Data["azure_client_id"]; !ok {
+		return nil, errors.New("azure_client_id does not exists")
+	}
+	creds.clientID = mysec.Data["azure_client_id"]
+
+	if _, ok := mysec.Data["azure_client_secret"]; !ok {
+		return nil, errors.New("azure_client_secret does not exists")
+	}
+	creds.clientSecret = mysec.Data["azure_client_secret"]
+
+	if _, ok := mysec.Data["azure_tenant_id"]; !ok {
+		return nil, errors.New("azure_tenant_id does not exists")
+	}
+	creds.tenantID = mysec.Data["azure_tenant_id"]
+
+	return &creds, nil
+}

pkg/operator/controllers/checker/checker_controller.go

@@ -10,6 +10,7 @@ import (
 	machinev1beta1 "github.com/openshift/machine-api-operator/pkg/apis/machine/v1beta1"
 	maoclient "github.com/openshift/machine-api-operator/pkg/generated/clientset/versioned"
 	"github.com/sirupsen/logrus"
+	"k8s.io/client-go/kubernetes"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/handler"
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
@@ -29,11 +30,11 @@ type CheckerController struct {
 	checkers []Checker
 }
 
-func NewReconciler(log *logrus.Entry, maocli maoclient.Interface, arocli aroclient.Interface, role string, deploymentMode deployment.Mode) *CheckerController {
+func NewReconciler(log *logrus.Entry, maocli maoclient.Interface, arocli aroclient.Interface, kubernetescli kubernetes.Interface, role string, deploymentMode deployment.Mode) *CheckerController {
 	checkers := []Checker{NewInternetChecker(log, arocli, role)}
 
 	if role == operator.RoleMaster {
-		checkers = append(checkers, NewMachineChecker(log, maocli, arocli, role, deploymentMode))
+		checkers = append(checkers, NewMachineChecker(log, maocli, arocli, role, deploymentMode), NewDNSChecker(log, arocli, kubernetescli, maocli, role))
 	}
 
 	return &CheckerController{

pkg/operator/controllers/checker/const.go

@@ -0,0 +1,10 @@
+package checker
+
+// Copyright (c) Microsoft Corporation.
+// Licensed under the Apache License 2.0.
+
+const (
+	azureCredentialSecretName      = "azure-credentials"
+	azureCredentialSecretNameSpace = "kube-system"
+	machineSetsNamespace           = "openshift-machine-api"
+)