diff --git a/api/v1alpha2/azurecluster_conversion.go b/api/v1alpha2/azurecluster_conversion.go index bf4a9e06a75..b7cfb45147b 100644 --- a/api/v1alpha2/azurecluster_conversion.go +++ b/api/v1alpha2/azurecluster_conversion.go @@ -61,7 +61,7 @@ func (src *AzureCluster) ConvertTo(dstRaw conversion.Hub) error { // nolint dst.Status.FailureDomains = restored.Status.FailureDomains dst.Spec.NetworkSpec.Vnet.CIDRBlocks = restored.Spec.NetworkSpec.Vnet.CIDRBlocks - dst.Spec.IdentityName = restored.Spec.IdentityName + dst.Spec.IdentityRef = restored.Spec.IdentityRef for _, restoredSubnet := range restored.Spec.NetworkSpec.Subnets { if restoredSubnet != nil { diff --git a/api/v1alpha2/zz_generated.conversion.go b/api/v1alpha2/zz_generated.conversion.go index 8fd316ef38b..e1be7a829ea 100644 --- a/api/v1alpha2/zz_generated.conversion.go +++ b/api/v1alpha2/zz_generated.conversion.go @@ -384,7 +384,7 @@ func autoConvert_v1alpha3_AzureClusterSpec_To_v1alpha2_AzureClusterSpec(in *v1al out.Location = in.Location // WARNING: in.ControlPlaneEndpoint requires manual conversion: does not exist in peer-type out.AdditionalTags = *(*Tags)(unsafe.Pointer(&in.AdditionalTags)) - // WARNING: in.IdentityName requires manual conversion: does not exist in peer-type + // WARNING: in.IdentityRef requires manual conversion: does not exist in peer-type return nil } diff --git a/api/v1alpha3/azurecluster_types.go b/api/v1alpha3/azurecluster_types.go index a44bc3921c0..88f4ff45b22 100644 --- a/api/v1alpha3/azurecluster_types.go +++ b/api/v1alpha3/azurecluster_types.go @@ -17,6 +17,7 @@ limitations under the License. package v1alpha3 import ( + corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" clusterv1 "sigs.k8s.io/cluster-api/api/v1alpha3" ) @@ -52,9 +53,9 @@ type AzureClusterSpec struct { // +optional AdditionalTags Tags `json:"additionalTags,omitempty"` - // IdentityName is a reference to a AzureIdentity to be used when reconciling this cluster + // IdentityRef is a reference to a AzureIdentity to be used when reconciling this cluster // +optional - IdentityName *string `json:"identityName,omitempty"` + IdentityRef *corev1.ObjectReference `json:"identityRef,omitempty"` } // AzureClusterStatus defines the observed state of AzureCluster diff --git a/api/v1alpha3/azureclusteridentity_types.go b/api/v1alpha3/azureclusteridentity_types.go new file mode 100644 index 00000000000..c8ea3a251dd --- /dev/null +++ b/api/v1alpha3/azureclusteridentity_types.go @@ -0,0 +1,105 @@ +/* +Copyright 2020 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package v1alpha3 + +import ( + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + clusterv1 "sigs.k8s.io/cluster-api/api/v1alpha3" +) + +// AzureClusterIdentitySpec defines the parameters that are used to create an AzureIdentity +type AzureClusterIdentitySpec struct { + // UserAssignedMSI or Service Principal + Type IdentityType `json:"type"` + // User assigned MSI resource id. + // +optional + ResourceID string `json:"resourceID,omitempty"` + // Both User Assigned MSI and SP can use this field. + ClientID string `json:"clientID"` + // ClientSecret is a secret reference which should contain either a Service Principal password or certificate secret. + // +optional + ClientSecret corev1.SecretReference `json:"clientSecret,omitempty"` + // Service principal primary tenant id. + TenantID string `json:"tenantID"` + // AllowedNamespaces is an array of namespaces that AzureClusters can + // use this Identity from. + // + // An empty list (default) indicates that AzureClusters can use this + // Identity from any namespace. This field is intentionally not a + // pointer because the nil behavior (no namespaces) is undesirable here. + // +optional + AllowedNamespaces []string `json:"allowedNamespaces"` +} + +// AzureClusterIdentityStatus defines the observed state of AzureClusterIdentity +type AzureClusterIdentityStatus struct { + // Conditions defines current service state of the AzureClusterIdentity. + // +optional + Conditions clusterv1.Conditions `json:"conditions,omitempty"` +} + +// +kubebuilder:object:root=true +// +kubebuilder:resource:path=azureclusteridentities,scope=Namespaced,categories=cluster-api +// +kubebuilder:storageversion +// +kubebuilder:subresource:status + +// AzureClusterIdentity is the Schema for the azureclustersidentities API +type AzureClusterIdentity struct { + metav1.TypeMeta `json:",inline"` + metav1.ObjectMeta `json:"metadata,omitempty"` + + Spec AzureClusterIdentitySpec `json:"spec,omitempty"` + Status AzureClusterIdentityStatus `json:"status,omitempty"` +} + +// +kubebuilder:object:root=true + +// AzureClusterIdentityList contains a list of AzureClusterIdentity +type AzureClusterIdentityList struct { + metav1.TypeMeta `json:",inline"` + metav1.ListMeta `json:"metadata,omitempty"` + Items []AzureClusterIdentity `json:"items"` +} + +// GetConditions returns the list of conditions for an AzureClusterIdentity API object. +func (c *AzureClusterIdentity) GetConditions() clusterv1.Conditions { + return c.Status.Conditions +} + +// SetConditions will set the given conditions on an AzureClusterIdentity object +func (c *AzureClusterIdentity) SetConditions(conditions clusterv1.Conditions) { + c.Status.Conditions = conditions +} + +// ClusterNamespaceAllowed indicates if the cluster namespace is allowed +func (c *AzureClusterIdentity) ClusterNamespaceAllowed(namespace string) bool { + if len(c.Spec.AllowedNamespaces) == 0 { + return true + } + for _, v := range c.Spec.AllowedNamespaces { + if v == namespace { + return true + } + } + + return false +} + +func init() { + SchemeBuilder.Register(&AzureClusterIdentity{}, &AzureClusterIdentityList{}) +} diff --git a/api/v1alpha3/conditions_consts.go b/api/v1alpha3/conditions_consts.go index 2cd86493b43..4328501d0e3 100644 --- a/api/v1alpha3/conditions_consts.go +++ b/api/v1alpha3/conditions_consts.go @@ -26,6 +26,8 @@ const ( LoadBalancerProvisioningReason = "LoadBalancerProvisioning" // LoadBalancerProvisioningFailedReason used for failure during provisioning of loadbalancer. LoadBalancerProvisioningFailedReason = "LoadBalancerProvisioningFailed" + // NamespaceNotAllowedByIdentity used to indicate cluster in a namespace not allowed by identity + NamespaceNotAllowedByIdentity = "NamespaceNotAllowedByIdentity" ) // AzureMachine Conditions and Reasons diff --git a/api/v1alpha3/types.go b/api/v1alpha3/types.go index 3343d23bf7b..cebf41eb459 100644 --- a/api/v1alpha3/types.go +++ b/api/v1alpha3/types.go @@ -330,6 +330,17 @@ const ( AzureIdentityBindingSelector = "capz-controller-aadpodidentity-selector" ) +// IdentityType represents different types of identities. +type IdentityType string + +const ( + // UserAssignedMSI represents a user-assigned identity. + UserAssignedMSI IdentityType = "UserAssignedMSI" + + // ServicePrincipal represents a service principal. + ServicePrincipal IdentityType = "ServicePrincipal" +) + // OSDisk defines the operating system disk for a VM. type OSDisk struct { OSType string `json:"osType"` diff --git a/api/v1alpha3/zz_generated.deepcopy.go b/api/v1alpha3/zz_generated.deepcopy.go index 8f5cff2e720..fd3d0111dc3 100644 --- a/api/v1alpha3/zz_generated.deepcopy.go +++ b/api/v1alpha3/zz_generated.deepcopy.go @@ -94,6 +94,108 @@ func (in *AzureCluster) DeepCopyObject() runtime.Object { return nil } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *AzureClusterIdentity) DeepCopyInto(out *AzureClusterIdentity) { + *out = *in + out.TypeMeta = in.TypeMeta + in.ObjectMeta.DeepCopyInto(&out.ObjectMeta) + in.Spec.DeepCopyInto(&out.Spec) + in.Status.DeepCopyInto(&out.Status) +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new AzureClusterIdentity. +func (in *AzureClusterIdentity) DeepCopy() *AzureClusterIdentity { + if in == nil { + return nil + } + out := new(AzureClusterIdentity) + in.DeepCopyInto(out) + return out +} + +// DeepCopyObject is an autogenerated deepcopy function, copying the receiver, creating a new runtime.Object. +func (in *AzureClusterIdentity) DeepCopyObject() runtime.Object { + if c := in.DeepCopy(); c != nil { + return c + } + return nil +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *AzureClusterIdentityList) DeepCopyInto(out *AzureClusterIdentityList) { + *out = *in + out.TypeMeta = in.TypeMeta + in.ListMeta.DeepCopyInto(&out.ListMeta) + if in.Items != nil { + in, out := &in.Items, &out.Items + *out = make([]AzureClusterIdentity, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new AzureClusterIdentityList. +func (in *AzureClusterIdentityList) DeepCopy() *AzureClusterIdentityList { + if in == nil { + return nil + } + out := new(AzureClusterIdentityList) + in.DeepCopyInto(out) + return out +} + +// DeepCopyObject is an autogenerated deepcopy function, copying the receiver, creating a new runtime.Object. +func (in *AzureClusterIdentityList) DeepCopyObject() runtime.Object { + if c := in.DeepCopy(); c != nil { + return c + } + return nil +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *AzureClusterIdentitySpec) DeepCopyInto(out *AzureClusterIdentitySpec) { + *out = *in + out.ClientSecret = in.ClientSecret + if in.AllowedNamespaces != nil { + in, out := &in.AllowedNamespaces, &out.AllowedNamespaces + *out = make([]string, len(*in)) + copy(*out, *in) + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new AzureClusterIdentitySpec. +func (in *AzureClusterIdentitySpec) DeepCopy() *AzureClusterIdentitySpec { + if in == nil { + return nil + } + out := new(AzureClusterIdentitySpec) + in.DeepCopyInto(out) + return out +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *AzureClusterIdentityStatus) DeepCopyInto(out *AzureClusterIdentityStatus) { + *out = *in + if in.Conditions != nil { + in, out := &in.Conditions, &out.Conditions + *out = make(apiv1alpha3.Conditions, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new AzureClusterIdentityStatus. +func (in *AzureClusterIdentityStatus) DeepCopy() *AzureClusterIdentityStatus { + if in == nil { + return nil + } + out := new(AzureClusterIdentityStatus) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *AzureClusterList) DeepCopyInto(out *AzureClusterList) { *out = *in @@ -138,9 +240,9 @@ func (in *AzureClusterSpec) DeepCopyInto(out *AzureClusterSpec) { (*out)[key] = val } } - if in.IdentityName != nil { - in, out := &in.IdentityName, &out.IdentityName - *out = new(string) + if in.IdentityRef != nil { + in, out := &in.IdentityRef, &out.IdentityRef + *out = new(v1.ObjectReference) **out = **in } } diff --git a/cloud/scope/cluster.go b/cloud/scope/cluster.go index f411c58349a..9efc4833970 100644 --- a/cloud/scope/cluster.go +++ b/cloud/scope/cluster.go @@ -61,13 +61,13 @@ func NewClusterScope(ctx context.Context, params ClusterScopeParams) (*ClusterSc params.Logger = klogr.New() } - if params.AzureCluster.Spec.IdentityName == nil { + if params.AzureCluster.Spec.IdentityRef == nil { err := params.AzureClients.setCredentials(params.AzureCluster.Spec.SubscriptionID) if err != nil { return nil, errors.Wrap(err, "failed to configure azure settings and credentials from environment") } } else { - credentailsProvider, err := NewAzureCredentialsProvider(ctx, params.Client, params.AzureCluster, to.String(params.AzureCluster.Spec.IdentityName), params.AzureCluster.Namespace) + credentailsProvider, err := NewAzureCredentialsProvider(ctx, params.Client, params.AzureCluster) if err != nil { return nil, errors.Wrap(err, "failed to init credentials provider") } diff --git a/cloud/scope/identity.go b/cloud/scope/identity.go index 9dd88f81592..d127796b7bf 100644 --- a/cloud/scope/identity.go +++ b/cloud/scope/identity.go @@ -24,6 +24,7 @@ import ( "github.com/Azure/go-autorest/autorest/adal" "github.com/pkg/errors" infrav1 "sigs.k8s.io/cluster-api-provider-azure/api/v1alpha3" + "sigs.k8s.io/cluster-api-provider-azure/util/identity" clusterv1 "sigs.k8s.io/cluster-api/api/v1alpha3" clusterctl "sigs.k8s.io/cluster-api/cmd/clusterctl/api/v1alpha3" "sigs.k8s.io/controller-runtime/pkg/client" @@ -37,40 +38,51 @@ import ( type AzureCredentialsProvider struct { Client client.Client AzureCluster *infrav1.AzureCluster - Identity *aadpodv1.AzureIdentity + Identity *infrav1.AzureClusterIdentity } // NewAzureCredentialsProvider creates a new AzureCredentialsProvider from the supplied inputs. -func NewAzureCredentialsProvider(ctx context.Context, kubeClient client.Client, azureCluster *infrav1.AzureCluster, identityName, namespace string) (*AzureCredentialsProvider, error) { - if identityName == "" { +func NewAzureCredentialsProvider(ctx context.Context, kubeClient client.Client, azureCluster *infrav1.AzureCluster) (*AzureCredentialsProvider, error) { + if azureCluster.Spec.IdentityRef == nil { return nil, errors.New("failed to generate new AzureCredentialsProvider from empty identityName") } - azureIdentity := &aadpodv1.AzureIdentity{} - err := kubeClient.Get(ctx, client.ObjectKey{Name: identityName, Namespace: namespace}, azureIdentity) - if err != nil { - return nil, errors.Wrap(err, "failed to get AzureIdentity") + ref := azureCluster.Spec.IdentityRef + // if the namespace isn't specified then assume it's in the same namespace as the AzureCluster + namespace := ref.Namespace + if namespace == "" { + namespace = azureCluster.Namespace + } + identity := &infrav1.AzureClusterIdentity{} + key := client.ObjectKey{Name: ref.Name, Namespace: namespace} + if err := kubeClient.Get(ctx, key, identity); err != nil { + return nil, errors.Errorf("failed to retrieve AzureClusterIdentity external object %q/%q: %v", key.Namespace, key.Name, err) } - if azureIdentity.Spec.Type != aadpodv1.ServicePrincipal { - return nil, errors.New("AzureIdentity is not of type Service Principal") + + if identity.Spec.Type != infrav1.ServicePrincipal { + return nil, errors.New("AzureClusterIdentity is not of type Service Principal") } return &AzureCredentialsProvider{ Client: kubeClient, AzureCluster: azureCluster, - Identity: azureIdentity, + Identity: identity, }, nil } // GetAuthorizer returns Azure authorizer based on the provided azure identity func (p *AzureCredentialsProvider) GetAuthorizer(ctx context.Context, resourceManagerEndpoint string) (autorest.Authorizer, error) { + azureIdentityType, err := getAzureIdentityType(p.Identity) + if err != nil { + return nil, err + } copiedIdentity := &aadpodv1.AzureIdentity{ TypeMeta: metav1.TypeMeta{ Kind: "AzureIdentity", APIVersion: "aadpodidentity.k8s.io/v1", }, ObjectMeta: metav1.ObjectMeta{ - Name: fmt.Sprintf("%s-%s", p.AzureCluster.Namespace, p.Identity.Name), + Name: identity.GetAzureIdentityName(p.AzureCluster.Name, p.AzureCluster.Namespace, p.Identity.Name), Namespace: infrav1.ControllerNamespace, Annotations: map[string]string{ aadpodv1.BehaviorKey: "namespaced", @@ -82,11 +94,17 @@ func (p *AzureCredentialsProvider) GetAuthorizer(ctx context.Context, resourceMa }, OwnerReferences: p.AzureCluster.OwnerReferences, }, - Spec: p.Identity.Spec, + Spec: aadpodv1.AzureIdentitySpec{ + Type: azureIdentityType, + TenantID: p.Identity.Spec.TenantID, + ClientID: p.Identity.Spec.ClientID, + ClientPassword: p.Identity.Spec.ClientSecret, + ResourceID: p.Identity.Spec.ResourceID, + }, } - err := p.Client.Create(ctx, copiedIdentity) + err = p.Client.Create(ctx, copiedIdentity) if err != nil && !apierrors.IsAlreadyExists(err) { - return nil, errors.Wrapf(err, "failed to create copied AzureIdentity %s in %s", copiedIdentity.Name, infrav1.ControllerNamespace) + return nil, errors.Errorf("failed to create copied AzureIdentity %s in %s: %v", copiedIdentity.Name, infrav1.ControllerNamespace, err) } azureIdentityBinding := &aadpodv1.AzureIdentityBinding{ @@ -111,22 +129,34 @@ func (p *AzureCredentialsProvider) GetAuthorizer(ctx context.Context, resourceMa } err = p.Client.Create(ctx, azureIdentityBinding) if err != nil && !apierrors.IsAlreadyExists(err) { - return nil, errors.Wrapf(err, "failed to create AzureIdentityBinding %s in %s", copiedIdentity.Name, infrav1.ControllerNamespace) + return nil, errors.Errorf("failed to create AzureIdentityBinding %s in %s: %v", copiedIdentity.Name, infrav1.ControllerNamespace, err) } var spt *adal.ServicePrincipalToken msiEndpoint, err := adal.GetMSIVMEndpoint() if err != nil { - return nil, errors.Wrap(err, "failed to get MSI endpoint") + return nil, errors.Errorf("failed to get MSI endpoint: %v", err) } - if p.Identity.Spec.Type == aadpodv1.ServicePrincipal { + if p.Identity.Spec.Type == infrav1.ServicePrincipal { spt, err = adal.NewServicePrincipalTokenFromMSIWithUserAssignedID(msiEndpoint, resourceManagerEndpoint, p.Identity.Spec.ClientID) if err != nil { - return nil, errors.Wrap(err, "failed to get token from service principal identity") + return nil, errors.Errorf("failed to get token from service principal identity: %v", err) } - } else if p.Identity.Spec.Type == aadpodv1.UserAssignedMSI { - return nil, errors.Wrap(err, "UserAssignedMSI not supported") + } else if p.Identity.Spec.Type == infrav1.UserAssignedMSI { + return nil, errors.Errorf("UserAssignedMSI not supported: %v", err) } return autorest.NewBearerAuthorizer(spt), nil } + +func getAzureIdentityType(identity *infrav1.AzureClusterIdentity) (aadpodv1.IdentityType, error) { + switch identity.Spec.Type { + case infrav1.ServicePrincipal: + return aadpodv1.ServicePrincipal, nil + case infrav1.UserAssignedMSI: + return aadpodv1.UserAssignedMSI, nil + } + + return 0, errors.New("AzureIdentity does not have a vaild type") + +} diff --git a/config/aadpodidentity/aad-pod-identity-deployment.yaml b/config/aadpodidentity/aad-pod-identity-deployment.yaml index ab372944c8b..dcdec1d12f9 100644 --- a/config/aadpodidentity/aad-pod-identity-deployment.yaml +++ b/config/aadpodidentity/aad-pod-identity-deployment.yaml @@ -3,8 +3,6 @@ apiVersion: apiextensions.k8s.io/v1beta1 kind: CustomResourceDefinition metadata: name: azureidentitybindings.aadpodidentity.k8s.io - labels: - clusterctl.cluster.x-k8s.io/move: true spec: group: aadpodidentity.k8s.io version: v1 @@ -17,8 +15,6 @@ apiVersion: apiextensions.k8s.io/v1beta1 kind: CustomResourceDefinition metadata: name: azureidentities.aadpodidentity.k8s.io - labels: - clusterctl.cluster.x-k8s.io/move: true spec: group: aadpodidentity.k8s.io version: v1 @@ -32,8 +28,6 @@ apiVersion: apiextensions.k8s.io/v1beta1 kind: CustomResourceDefinition metadata: name: azurepodidentityexceptions.aadpodidentity.k8s.io - labels: - clusterctl.cluster.x-k8s.io/move: true spec: group: aadpodidentity.k8s.io version: v1 diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_azureclusteridentities.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_azureclusteridentities.yaml new file mode 100644 index 00000000000..89796776ea9 --- /dev/null +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_azureclusteridentities.yaml @@ -0,0 +1,141 @@ + +--- +apiVersion: apiextensions.k8s.io/v1 +kind: CustomResourceDefinition +metadata: + annotations: + controller-gen.kubebuilder.io/version: v0.3.0 + creationTimestamp: null + name: azureclusteridentities.infrastructure.cluster.x-k8s.io +spec: + group: infrastructure.cluster.x-k8s.io + names: + categories: + - cluster-api + kind: AzureClusterIdentity + listKind: AzureClusterIdentityList + plural: azureclusteridentities + singular: azureclusteridentity + scope: Namespaced + versions: + - name: v1alpha3 + schema: + openAPIV3Schema: + description: AzureClusterIdentity is the Schema for the azureclustersidentities + API + properties: + apiVersion: + description: 'APIVersion defines the versioned schema of this representation + of an object. Servers should convert recognized schemas to the latest + internal value, and may reject unrecognized values. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources' + type: string + kind: + description: 'Kind is a string value representing the REST resource this + object represents. Servers may infer this from the endpoint the client + submits requests to. Cannot be updated. In CamelCase. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds' + type: string + metadata: + type: object + spec: + description: AzureClusterIdentitySpec defines the parameters that are + used to create an AzureIdentity + properties: + allowedNamespaces: + description: "AllowedNamespaces is an array of namespaces that AzureClusters + can use this Identity from. \n An empty list (default) indicates + that AzureClusters can use this Identity from any namespace. This + field is intentionally not a pointer because the nil behavior (no + namespaces) is undesirable here." + items: + type: string + type: array + clientID: + description: Both User Assigned MSI and SP can use this field. + type: string + clientSecret: + description: ClientSecret is a secret reference which should contain + either a Service Principal password or certificate secret. + properties: + name: + description: Name is unique within a namespace to reference a + secret resource. + type: string + namespace: + description: Namespace defines the space within which the secret + name must be unique. + type: string + type: object + resourceID: + description: User assigned MSI resource id. + type: string + tenantID: + description: Service principal primary tenant id. + type: string + type: + description: UserAssignedMSI or Service Principal + type: string + required: + - clientID + - tenantID + - type + type: object + status: + description: AzureClusterIdentityStatus defines the observed state of + AzureClusterIdentity + properties: + conditions: + description: Conditions defines current service state of the AzureClusterIdentity. + items: + description: Condition defines an observation of a Cluster API resource + operational state. + properties: + lastTransitionTime: + description: Last time the condition transitioned from one status + to another. This should be when the underlying condition changed. + If that is not known, then using the time when the API field + changed is acceptable. + format: date-time + type: string + message: + description: A human readable message indicating details about + the transition. This field may be empty. + type: string + reason: + description: The reason for the condition's last transition + in CamelCase. The specific API may choose whether or not this + field is considered a guaranteed API. This field may not be + empty. + type: string + severity: + description: Severity provides an explicit classification of + Reason code, so the users or machines can immediately understand + the current situation and act accordingly. The Severity field + MUST be set only when Status=False. + type: string + status: + description: Status of the condition, one of True, False, Unknown. + type: string + type: + description: Type of condition in CamelCase or in foo.example.com/CamelCase. + Many .condition.type values are consistent across resources + like Available, but because arbitrary conditions can be useful + (see .node.status.conditions), the ability to deconflict is + important. + type: string + required: + - status + - type + type: object + type: array + type: object + type: object + served: true + storage: true + subresources: + status: {} +status: + acceptedNames: + kind: "" + plural: "" + conditions: [] + storedVersions: [] diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_azureclusters.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_azureclusters.yaml index 35dfffb669f..aaf7082d5e1 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_azureclusters.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_azureclusters.yaml @@ -459,10 +459,43 @@ spec: - host - port type: object - identityName: - description: IdentityName is a reference to a AzureIdentity to be - used when reconciling this cluster - type: string + identityRef: + description: IdentityRef is a reference to a AzureIdentity to be used + when reconciling this cluster + properties: + apiVersion: + description: API version of the referent. + type: string + fieldPath: + description: 'If referring to a piece of an object instead of + an entire object, this string should contain a valid JSON/Go + field access statement, such as desiredState.manifest.containers[2]. + For example, if the object reference is to a container within + a pod, this would take on a value like: "spec.containers{name}" + (where "name" refers to the name of the container that triggered + the event) or if no container name is specified "spec.containers[2]" + (container with index 2 in this pod). This syntax is chosen + only to have some well-defined way of referencing a part of + an object. TODO: this design is not final and this field is + subject to change in the future.' + type: string + kind: + description: 'Kind of the referent. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds' + type: string + name: + description: 'Name of the referent. More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names' + type: string + namespace: + description: 'Namespace of the referent. More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/namespaces/' + type: string + resourceVersion: + description: 'Specific resourceVersion to which this reference + is made, if any. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#concurrency-control-and-consistency' + type: string + uid: + description: 'UID of the referent. More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#uids' + type: string + type: object location: type: string networkSpec: diff --git a/config/crd/kustomization.yaml b/config/crd/kustomization.yaml index 8cf3f4ded60..5eef750fd75 100644 --- a/config/crd/kustomization.yaml +++ b/config/crd/kustomization.yaml @@ -8,6 +8,7 @@ resources: - bases/infrastructure.cluster.x-k8s.io_azuremachines.yaml - bases/infrastructure.cluster.x-k8s.io_azureclusters.yaml - bases/infrastructure.cluster.x-k8s.io_azuremachinetemplates.yaml + - bases/infrastructure.cluster.x-k8s.io_azureclusteridentities.yaml - bases/exp.infrastructure.cluster.x-k8s.io_azuremachinepools.yaml - bases/exp.infrastructure.cluster.x-k8s.io_azuremanagedmachinepools.yaml - bases/exp.infrastructure.cluster.x-k8s.io_azuremanagedclusters.yaml diff --git a/config/rbac/role.yaml b/config/rbac/role.yaml index c726877fd4d..fc93ac6f65f 100644 --- a/config/rbac/role.yaml +++ b/config/rbac/role.yaml @@ -163,6 +163,19 @@ rules: - get - patch - update +- apiGroups: + - infrastructure.cluster.x-k8s.io + resources: + - azureclusteridentities + - azureclusteridentities/status + verbs: + - create + - delete + - get + - list + - patch + - update + - watch - apiGroups: - infrastructure.cluster.x-k8s.io resources: diff --git a/controllers/azurecluster_controller.go b/controllers/azurecluster_controller.go index 883e5de9d48..c7bb726d38c 100644 --- a/controllers/azurecluster_controller.go +++ b/controllers/azurecluster_controller.go @@ -32,6 +32,7 @@ import ( "sigs.k8s.io/cluster-api/util" "sigs.k8s.io/cluster-api/util/annotations" "sigs.k8s.io/cluster-api/util/conditions" + "sigs.k8s.io/cluster-api/util/patch" "sigs.k8s.io/cluster-api/util/predicates" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" @@ -85,6 +86,7 @@ func (r *AzureClusterReconciler) SetupWithManager(mgr ctrl.Manager, options cont // +kubebuilder:rbac:groups=infrastructure.cluster.x-k8s.io,resources=azureclusters/status,verbs=get;update;patch // +kubebuilder:rbac:groups=cluster.x-k8s.io,resources=clusters;clusters/status,verbs=get;list;watch // +kubebuilder:rbac:groups=infrastructure.cluster.x-k8s.io,resources=azuremachinetemplates;azuremachinetemplates/status,verbs=get;list;watch +// +kubebuilder:rbac:groups=infrastructure.cluster.x-k8s.io,resources=azureclusteridentities;azureclusteridentities/status,verbs=get;list;watch;create;update;patch;delete // Reconcile idempotently gets, creates, and updates a cluster. func (r *AzureClusterReconciler) Reconcile(req ctrl.Request) (_ ctrl.Result, reterr error) { @@ -175,6 +177,27 @@ func (r *AzureClusterReconciler) reconcileNormal(ctx context.Context, clusterSco return reconcile.Result{}, err } + if azureCluster.Spec.IdentityRef != nil { + identity, err := GetClusterIdentityFromRef(ctx, clusterScope.Client, azureCluster.Namespace, azureCluster.Spec.IdentityRef) + if err != nil { + return reconcile.Result{}, err + } + if !identity.ClusterNamespaceAllowed(azureCluster.Namespace) { + conditions.MarkFalse(azureCluster, infrav1.NetworkInfrastructureReadyCondition, infrav1.NamespaceNotAllowedByIdentity, clusterv1.ConditionSeverityError, "") + return reconcile.Result{}, errors.New("AzureClusterIdentity list of allowed namespaces doesn't include current cluster namespace") + } + if identity.Namespace == azureCluster.Namespace { + patchhelper, err := patch.NewHelper(identity, r.Client) + if err != nil { + return reconcile.Result{}, errors.Wrap(err, "failed to init patch helper") + } + identity.ObjectMeta.OwnerReferences = azureCluster.GetOwnerReferences() + if err := patchhelper.Patch(ctx, identity); err != nil { + return reconcile.Result{}, err + } + } + } + // Handle backcompat for CidrBlock if clusterScope.Vnet().CidrBlock != "" { message := "vnet cidrBlock is deprecated, use cidrBlocks instead" diff --git a/controllers/azureidentity_controller.go b/controllers/azureidentity_controller.go index 4522f5d86dc..365bb9a259c 100644 --- a/controllers/azureidentity_controller.go +++ b/controllers/azureidentity_controller.go @@ -21,7 +21,6 @@ import ( "time" aadpodv1 "github.com/Azure/aad-pod-identity/pkg/apis/aadpodidentity/v1" - "github.com/Azure/go-autorest/autorest/to" "github.com/go-logr/logr" "github.com/pkg/errors" "go.opentelemetry.io/otel/api/trace" @@ -30,6 +29,7 @@ import ( apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/client-go/tools/record" infrav1 "sigs.k8s.io/cluster-api-provider-azure/api/v1alpha3" + "sigs.k8s.io/cluster-api-provider-azure/util/identity" "sigs.k8s.io/cluster-api-provider-azure/util/reconciler" "sigs.k8s.io/cluster-api-provider-azure/util/tele" clusterv1 "sigs.k8s.io/cluster-api/api/v1alpha3" @@ -53,7 +53,7 @@ type AzureIdentityReconciler struct { // SetupWithManager initializes this controller with a manager func (r *AzureIdentityReconciler) SetupWithManager(mgr ctrl.Manager, options controller.Options) error { - log := r.Log.WithValues("controller", "AzureCluster") + log := r.Log.WithValues("controller", "AzureIdentity") c, err := ctrl.NewControllerManagedBy(mgr). WithOptions(options). For(&infrav1.AzureCluster{}). @@ -135,7 +135,8 @@ func (r *AzureIdentityReconciler) Reconcile(req ctrl.Request) (_ ctrl.Result, re } } - if binding.Spec.AzureIdentity != to.String(azCluster.Spec.IdentityName) { + expectedIdentityName := identity.GetAzureIdentityName(azCluster.Name, azCluster.Namespace, azCluster.Spec.IdentityRef.Name) + if binding.Spec.AzureIdentity != expectedIdentityName { bindingsToDelete = append(bindingsToDelete, b) } } diff --git a/controllers/helpers.go b/controllers/helpers.go index e4cf6dad3f3..6aa18ba73d7 100644 --- a/controllers/helpers.go +++ b/controllers/helpers.go @@ -387,3 +387,20 @@ func ShouldDeleteIndividualResources(ctx context.Context, clusterScope *scope.Cl // Instead, take the long way and delete all resources one by one. return err != nil || !managed } + +// GetClusterIdentityFromRef returns the AzureClusterIdentity referenced by the AzureCluster +func GetClusterIdentityFromRef(ctx context.Context, c client.Client, azureClusterNamespace string, ref *corev1.ObjectReference) (*infrav1.AzureClusterIdentity, error) { + identity := &infrav1.AzureClusterIdentity{} + if ref != nil { + namespace := ref.Namespace + if namespace == "" { + namespace = azureClusterNamespace + } + key := client.ObjectKey{Name: ref.Name, Namespace: namespace} + if err := c.Get(ctx, key, identity); err != nil { + return nil, err + } + return identity, nil + } + return nil, nil +} diff --git a/controllers/helpers_test.go b/controllers/helpers_test.go index cda0472991d..1e59a9a70a3 100644 --- a/controllers/helpers_test.go +++ b/controllers/helpers_test.go @@ -138,7 +138,7 @@ func TestGetCloudProviderConfig(t *testing.T) { for name, tc := range cases { t.Run(name, func(t *testing.T) { - clusterScope, err := scope.NewClusterScope(scope.ClusterScopeParams{ + clusterScope, err := scope.NewClusterScope(context.Background(), scope.ClusterScopeParams{ AzureClients: scope.AzureClients{ Authorizer: autorest.NullAuthorizer{}, }, diff --git a/docs/book/src/topics/multitenancy.md b/docs/book/src/topics/multitenancy.md index 891f63c6999..3ba2302aea5 100644 --- a/docs/book/src/topics/multitenancy.md +++ b/docs/book/src/topics/multitenancy.md @@ -6,18 +6,27 @@ This is achieved using the [aad-pod-identity](https://azure.github.io/aad-pod-id ## Service Principal Identity -Once a new SP Identity is created in Azure, a new resource of [AzureIdentity](https://azure.github.io/aad-pod-identity/docs/concepts/azureidentity/) should be created in the managment cluster, for example +Once a new SP Identity is created in Azure, the coresponding value should be added to the AzureCluster expected to use that identity, for example ```yaml -apiVersion: "aadpodidentity.k8s.io/v1" -kind: AzureIdentity +apiVersion: infrastructure.cluster.x-k8s.io/v1alpha3 +kind: AzureCluster metadata: - name: + name: example-cluster + namespace: default spec: - type: 1 - tenantID: - clientID: - clientPassword: {"name":"","namespace":"default"} + location: eastus + networkSpec: + vnet: + name: example-cluster-vnet + resourceGroup: example-cluster + subscriptionID: + identitySpec: + name: + type: ServicePrincipal + tenantID: + clientID: + clientSecret: {"name":"","namespace":"default"} ``` The password will need to be added in a secret similar to the following example @@ -45,24 +54,6 @@ data: password: PASSWORD ``` -and then this identity name will be added to the Azure Cluster - -```yaml -apiVersion: infrastructure.cluster.x-k8s.io/v1alpha3 -kind: AzureCluster -metadata: - name: example-cluster - namespace: default -spec: - location: eastus - networkSpec: - vnet: - name: example-cluster-vnet - resourceGroup: example-cluster - subscriptionID: - identityName: -``` - for more details on how aad-pod-identity works, please check the guide [here](https://azure.github.io/aad-pod-identity/docs/) ## User Assiged Identity diff --git a/templates/cluster-template-multi-tenancy.yaml b/templates/cluster-template-multi-tenancy.yaml index 014caa513cf..42abb714c8a 100644 --- a/templates/cluster-template-multi-tenancy.yaml +++ b/templates/cluster-template-multi-tenancy.yaml @@ -25,7 +25,11 @@ metadata: name: ${CLUSTER_NAME} namespace: default spec: - identityName: ${CLUSTER_IDENTITY_NAME} + identityRef: + apiVersion: infrastructure.cluster.x-k8s.io/v1alpha3 + kind: AzureClusterIdentity + name: ${CLUSTER_IDENTITY_NAME} + namespace: ${CLUSTER_IDENTITY_NAMESPACE} location: ${AZURE_LOCATION} networkSpec: vnet: @@ -198,3 +202,16 @@ spec: cloud-provider: azure name: '{{ ds.meta_data["local_hostname"] }}' useExperimentalRetryJoin: true +--- +apiVersion: infrastructure.cluster.x-k8s.io/v1alpha3 +kind: AzureClusterIdentity +metadata: + name: ${CLUSTER_IDENTITY_NAME} + namespace: default +spec: + clientID: ${AZURE_CLUSTER_IDENTITY_CLIENT_ID} + clientSecret: + name: ${AZURE_CLUSTER_IDENTITY_SECRET_NAME} + namespace: ${AZURE_CLUSTER_IDENTITY_SECRET_NAMESPACE} + tenantID: ${AZURE_TENANT_ID} + type: ServicePrincipal diff --git a/templates/flavors/multi-tenancy/azure-cluster-identity.yaml b/templates/flavors/multi-tenancy/azure-cluster-identity.yaml new file mode 100644 index 00000000000..9a77568c663 --- /dev/null +++ b/templates/flavors/multi-tenancy/azure-cluster-identity.yaml @@ -0,0 +1,10 @@ +--- +apiVersion: infrastructure.cluster.x-k8s.io/v1alpha3 +kind: AzureClusterIdentity +metadata: + name: "${CLUSTER_IDENTITY_NAME}" +spec: + type: ServicePrincipal + tenantID: "${AZURE_TENANT_ID}" + clientID: "${AZURE_CLUSTER_IDENTITY_CLIENT_ID}" + clientSecret: {"name":"${AZURE_CLUSTER_IDENTITY_SECRET_NAME}","namespace":"${AZURE_CLUSTER_IDENTITY_SECRET_NAMESPACE}"} diff --git a/templates/flavors/multi-tenancy/kustomization.yaml b/templates/flavors/multi-tenancy/kustomization.yaml index b92ffcedd7e..047db53df8b 100644 --- a/templates/flavors/multi-tenancy/kustomization.yaml +++ b/templates/flavors/multi-tenancy/kustomization.yaml @@ -1,5 +1,6 @@ namespace: default resources: - ../default + - azure-cluster-identity.yaml patchesStrategicMerge: - - patches/sp-multitenancy-identity.yaml + - patches/azurecluster-identity-ref.yaml diff --git a/templates/flavors/multi-tenancy/patches/azurecluster-identity-ref.yaml b/templates/flavors/multi-tenancy/patches/azurecluster-identity-ref.yaml new file mode 100644 index 00000000000..e2323439139 --- /dev/null +++ b/templates/flavors/multi-tenancy/patches/azurecluster-identity-ref.yaml @@ -0,0 +1,12 @@ +--- +apiVersion: infrastructure.cluster.x-k8s.io/v1alpha3 +kind: AzureCluster +metadata: + name: ${CLUSTER_NAME} +spec: + identityRef: + apiVersion: infrastructure.cluster.x-k8s.io/v1alpha3 + kind: AzureClusterIdentity + name: "${CLUSTER_IDENTITY_NAME}" + namespace: "${CLUSTER_IDENTITY_NAMESPACE}" + diff --git a/templates/flavors/multi-tenancy/patches/sp-multitenancy-identity.yaml b/templates/flavors/multi-tenancy/patches/sp-multitenancy-identity.yaml deleted file mode 100644 index fb3ae10012c..00000000000 --- a/templates/flavors/multi-tenancy/patches/sp-multitenancy-identity.yaml +++ /dev/null @@ -1,8 +0,0 @@ ---- -apiVersion: infrastructure.cluster.x-k8s.io/v1alpha3 -kind: AzureCluster -metadata: - name: ${CLUSTER_NAME} -spec: - identityName: ${CLUSTER_IDENTITY_NAME} - \ No newline at end of file diff --git a/templates/test/cluster-template-prow-multi-tenancy.yaml b/templates/test/cluster-template-prow-multi-tenancy.yaml index 7666e7ea082..839b1d40090 100644 --- a/templates/test/cluster-template-prow-multi-tenancy.yaml +++ b/templates/test/cluster-template-prow-multi-tenancy.yaml @@ -28,7 +28,11 @@ spec: additionalTags: creationTimestamp: ${TIMESTAMP} jobName: ${JOB_NAME} - identityName: ${CLUSTER_IDENTITY_NAME} + identityRef: + apiVersion: infrastructure.cluster.x-k8s.io/v1alpha3 + kind: AzureClusterIdentity + name: ${CLUSTER_IDENTITY_NAME} + namespace: ${CLUSTER_IDENTITY_NAMESPACE} location: ${AZURE_LOCATION} networkSpec: vnet: @@ -202,6 +206,19 @@ spec: name: '{{ ds.meta_data["local_hostname"] }}' useExperimentalRetryJoin: true --- +apiVersion: infrastructure.cluster.x-k8s.io/v1alpha3 +kind: AzureClusterIdentity +metadata: + name: ${CLUSTER_IDENTITY_NAME} + namespace: default +spec: + clientID: ${AZURE_CLUSTER_IDENTITY_CLIENT_ID} + clientSecret: + name: ${AZURE_CLUSTER_IDENTITY_SECRET_NAME} + namespace: ${AZURE_CLUSTER_IDENTITY_SECRET_NAMESPACE} + tenantID: ${AZURE_TENANT_ID} + type: ServicePrincipal +--- apiVersion: v1 data: ${CNI_RESOURCES_IPV6} kind: ConfigMap diff --git a/test/e2e/azure_test.go b/test/e2e/azure_test.go index 75a17cb306e..fa03f54c714 100644 --- a/test/e2e/azure_test.go +++ b/test/e2e/azure_test.go @@ -25,13 +25,11 @@ import ( "path/filepath" "time" - aadpodv1 "github.com/Azure/aad-pod-identity/pkg/apis/aadpodidentity/v1" . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/utils/pointer" - infrav1 "sigs.k8s.io/cluster-api-provider-azure/api/v1alpha3" clusterv1 "sigs.k8s.io/cluster-api/api/v1alpha3" capi_e2e "sigs.k8s.io/cluster-api/test/e2e" "sigs.k8s.io/cluster-api/test/framework/clusterctl" @@ -356,12 +354,7 @@ var _ = Describe("Workload cluster creation", func() { Context("Creating a cluster using a different SP identity", func() { BeforeEach(func() { - tenantID := os.Getenv("AZURE_TENANT_ID") - spClientID := os.Getenv("AZURE_MULTI_TENANCY_ID") spClientSecret := os.Getenv("AZURE_MULTI_TENANCY_SECRET") - identityName := e2eConfig.GetVariable(MultiTenancyIdentityName) - os.Setenv("CLUSTER_IDENTITY_NAME", identityName) - secret := &corev1.Secret{ ObjectMeta: metav1.ObjectMeta{ Name: "sp-identity-secret", @@ -372,38 +365,17 @@ var _ = Describe("Workload cluster creation", func() { } err := bootstrapClusterProxy.GetClient().Create(ctx, secret) Expect(err).ToNot(HaveOccurred()) - - azureIdentity := &aadpodv1.AzureIdentity{ - TypeMeta: metav1.TypeMeta{ - Kind: "AzureIdentity", - APIVersion: "aadpodidentity.k8s.io/v1", - }, - ObjectMeta: metav1.ObjectMeta{ - Name: identityName, - Namespace: namespace.Name, - Annotations: map[string]string{ - aadpodv1.BehaviorKey: "namespaced", - }, - Labels: map[string]string{ - clusterv1.ClusterLabelName: clusterName, - infrav1.ClusterLabelNamespace: namespace.Name, - }, - }, - Spec: aadpodv1.AzureIdentitySpec{ - Type: 1, - TenantID: tenantID, - ClientID: spClientID, - ClientPassword: corev1.SecretReference{ - Name: "sp-identity-secret", - Namespace: namespace.Name, - }, - }, - } - err = bootstrapClusterProxy.GetClient().Create(ctx, azureIdentity) - Expect(err).ToNot(HaveOccurred()) }) It("with a single control plane node and 1 node", func() { + spClientID := os.Getenv("AZURE_MULTI_TENANCY_ID") + identityName := e2eConfig.GetVariable(MultiTenancyIdentityName) + os.Setenv("CLUSTER_IDENTITY_NAME", identityName) + os.Setenv("CLUSTER_IDENTITY_NAMESPACE", namespace.Name) + os.Setenv("AZURE_CLUSTER_IDENTITY_CLIENT_ID", spClientID) + os.Setenv("AZURE_CLUSTER_IDENTITY_SECRET_NAME", "sp-identity-secret") + os.Setenv("AZURE_CLUSTER_IDENTITY_SECRET_NAMESPACE", namespace.Name) + result := clusterctl.ApplyClusterTemplateAndWait(ctx, clusterctl.ApplyClusterTemplateAndWaitInput{ ClusterProxy: bootstrapClusterProxy, ConfigCluster: clusterctl.ConfigClusterInput{ diff --git a/util/identity/defaults.go b/util/identity/defaults.go new file mode 100644 index 00000000000..940cd9667fa --- /dev/null +++ b/util/identity/defaults.go @@ -0,0 +1,24 @@ +/* +Copyright 2020 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package identity + +import "fmt" + +// GetAzureIdentityName formats the name of the AzureIdentity created by he capz controller +func GetAzureIdentityName(clusterName, clusterNamespace, identityName string) string { + return fmt.Sprintf("%s-%s-%s", clusterName, clusterNamespace, identityName) +}