-
Notifications
You must be signed in to change notification settings - Fork 25
Adding FIC Code #112
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Adding FIC Code #112
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -122,6 +122,11 @@ type AuthenticationMethod struct { | |
|
|
||
| // WorkloadIdentity uses Azure Workload Identity to authenticate with Azure. | ||
| WorkloadIdentity *WorkloadIdentityAuth `json:"workloadIdentity,omitempty"` | ||
|
|
||
| // +kubebuilder:validation:Optional | ||
|
|
||
| // Federated Identity Creationals for Cross Cloud | ||
| FederatedIdentity *FederatedIdentityAuth `json:"federatedIdentity,omitempty"` | ||
| } | ||
|
|
||
| // +kubebuilder:validation:XValidation:rule="[has(self.clientID), has(self.resourceID)].exists_one(x, x)", message="only client or resource ID can be set" | ||
|
|
@@ -171,6 +176,64 @@ type WorkloadIdentityAuth struct { | |
| TenantID string `json:"tenantID,omitempty"` | ||
| } | ||
|
|
||
| // +kubebuilder:validation:XValidation:rule="(has(self.sourceClientID) && has(self.SourceTenantID))", message="custom client and tenant identifiers must be provided together, if at all" | ||
|
|
||
| type FederatedIdentityAuth struct { | ||
| // +kubebuilder:validation:Required | ||
| // +kubebuilder:example="1b461305-28be-5271-beda-bd9fd2e24251" | ||
|
|
||
| // SourceClienTid holds the identified of identity residing on the tenant where request has been made. | ||
| SourceClientID string `json:"sourceClientID,omitempty"` | ||
|
|
||
| // +kubebuilder:validation:Required | ||
| // +kubebuilder:example="72f988bf-86f1-41af-91ab-2d7cd011db47" | ||
|
|
||
| // TenantID holds an optional tenant identifier of a federated identity. | ||
| // Specify this identifier if multiple identities are federated with the | ||
| // service account and the identity to use for image pulling is not the | ||
| // default identity stored in the service account's annotations. The | ||
| // client and tenant ID must be specified together. | ||
| SourceTenantID string `json:"sourceTenantID,omitempty"` | ||
|
|
||
| // +kubebuilder:validation:Required | ||
| // +kubebuilder:example="1b461305-28be-5271-beda-bd9fd2e24251" | ||
|
|
||
| // ClientID holds an optional client identifier of a federated identity. | ||
| // Specify this identifier if multiple identities are federated with the | ||
| // service account and the identity to use for image pulling is not the | ||
| // default identity stored in the service account's annotations. The | ||
| // client and tenant ID must be specified together. | ||
| TargetClientID string `json:"targetClientID,omitempty"` | ||
|
|
||
| // +kubebuilder:validation:Required | ||
| // +kubebuilder:example="72f988bf-86f1-41af-91ab-2d7cd011db47" | ||
|
|
||
| // TenantID holds an optional tenant identifier of a federated identity. | ||
| // Specify this identifier if multiple identities are federated with the | ||
| // service account and the identity to use for image pulling is not the | ||
| // default identity stored in the service account's annotations. The | ||
| // client and tenant ID must be specified together. | ||
| TargetTenantID string `json:"targetTenantID,omitempty"` | ||
|
|
||
| // +kubebuilder:validation:Required | ||
| // +kubebuilder:example="72f988bf-86f1-41af-91ab-2d7cd011db47" | ||
|
|
||
| // TenantID holds an optional tenant identifier of a federated identity. | ||
| // Specify this identifier if multiple identities are federated with the | ||
| // service account and the identity to use for image pulling is not the | ||
| // default identity stored in the service account's annotations. The | ||
| // client and tenant ID must be specified together. | ||
| Scope string `json:"scope,omitempty"` | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: Missing comment, it's same as for tenantid |
||
|
|
||
| // +kubebuilder:validation:Required | ||
| // +kubebuilder:validation:Enum=PublicCloud;USGovernmentCloud;ChinaCloud;AirgappedCloud | ||
| // +kubebuilder:default=PublicCloud | ||
| // +kubebuilder:example=PublicCloud | ||
|
|
||
| // Environment specifies the Azure Cloud environment in which the ACR is deployed. | ||
| TargetEnvironment AzureEnvironmentType `json:"targetEnvironment,omitempty"` | ||
| } | ||
|
|
||
| // AcrPullBindingStatus defines the observed state of AcrPullBinding | ||
| type AcrPullBindingStatus struct { | ||
| // +kubebuilder:validation:Optional | ||
|
|
||
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -139,6 +139,66 @@ spec: | |
| description: Auth determines how we will authenticate to the Azure | ||
| Container Registry. Only one method may be provided. | ||
| properties: | ||
| federatedIdentity: | ||
| description: Federated Identity Creationals for Cross Cloud | ||
| properties: | ||
| scope: | ||
| description: |- | ||
| TenantID holds an optional tenant identifier of a federated identity. | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this description correct. This is the target scope right? |
||
| Specify this identifier if multiple identities are federated with the | ||
| service account and the identity to use for image pulling is not the | ||
| default identity stored in the service account's annotations. The | ||
| client and tenant ID must be specified together. | ||
| example: 72f988bf-86f1-41af-91ab-2d7cd011db47 | ||
| type: string | ||
| sourceClientID: | ||
| description: SourceClienTid holds the identified of identity | ||
| residing on the tenant where request has been made. | ||
| example: 1b461305-28be-5271-beda-bd9fd2e24251 | ||
| type: string | ||
| sourceTenantID: | ||
| description: |- | ||
| TenantID holds an optional tenant identifier of a federated identity. | ||
| Specify this identifier if multiple identities are federated with the | ||
| service account and the identity to use for image pulling is not the | ||
| default identity stored in the service account's annotations. The | ||
| client and tenant ID must be specified together. | ||
| example: 72f988bf-86f1-41af-91ab-2d7cd011db47 | ||
| type: string | ||
| targetClientID: | ||
| description: |- | ||
| ClientID holds an optional client identifier of a federated identity. | ||
| Specify this identifier if multiple identities are federated with the | ||
| service account and the identity to use for image pulling is not the | ||
| default identity stored in the service account's annotations. The | ||
| client and tenant ID must be specified together. | ||
| example: 1b461305-28be-5271-beda-bd9fd2e24251 | ||
| type: string | ||
| targetEnvironment: | ||
| default: PublicCloud | ||
| description: Environment specifies the Azure Cloud environment | ||
| in which the ACR is deployed. | ||
| enum: | ||
| - PublicCloud | ||
| - USGovernmentCloud | ||
| - ChinaCloud | ||
| - AirgappedCloud | ||
| example: PublicCloud | ||
| type: string | ||
| targetTenantID: | ||
| description: |- | ||
| TenantID holds an optional tenant identifier of a federated identity. | ||
| Specify this identifier if multiple identities are federated with the | ||
| service account and the identity to use for image pulling is not the | ||
| default identity stored in the service account's annotations. The | ||
| client and tenant ID must be specified together. | ||
| example: 72f988bf-86f1-41af-91ab-2d7cd011db47 | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Better to make this AME or prdtrs01 right? Those are the only allowed combinations when originating from a ME/trs tenant. |
||
| type: string | ||
| type: object | ||
| x-kubernetes-validations: | ||
| - message: custom client and tenant identifiers must be provided | ||
| together, if at all | ||
| rule: (has(self.sourceClientID) && has(self.SourceTenantID)) | ||
| managedIdentity: | ||
| description: ManagedIdentity uses Azure Managed Identity to authenticate | ||
| with Azure. | ||
|
|
||
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -10,6 +10,8 @@ | |
| "github.com/Azure/azure-sdk-for-go/sdk/azcore/policy" | ||
| "github.com/Azure/azure-sdk-for-go/sdk/azidentity" | ||
|
|
||
| ctrl "sigs.k8s.io/controller-runtime" | ||
|
Check failure on line 13 in pkg/authorizer/token_retriever.go
|
||
|
|
||
| msiacrpullv1beta2 "github.com/Azure/msi-acrpull/api/v1beta2" | ||
| ) | ||
|
|
||
|
|
@@ -36,6 +38,7 @@ | |
|
|
||
| var credential azcore.TokenCredential | ||
| var err error | ||
|
|
||
| switch { | ||
| case spec.Auth.ManagedIdentity != nil: | ||
| var id azidentity.ManagedIDKind | ||
|
|
@@ -45,7 +48,7 @@ | |
| id = azidentity.ResourceID(spec.Auth.ManagedIdentity.ResourceID) | ||
| } | ||
| credential, err = azidentity.NewManagedIdentityCredential(&azidentity.ManagedIdentityCredentialOptions{ID: id}) | ||
| case spec.Auth.WorkloadIdentity != nil: | ||
| case spec.Auth.WorkloadIdentity != nil || spec.Auth.FederatedIdentity != nil: | ||
| // n.b. the built-in azidentity.WorkloadIdentityCredential assumes we're loading a service account token | ||
| // from a file in a Pod, where the Kubernetes API server is rotating it, etc. Unfortunately that is not | ||
| // our use-case here, and we certainly don't want to centralize every service account token we ever mint | ||
|
|
@@ -68,7 +71,15 @@ | |
| // this should never happen with the validation we have on the CRD | ||
| panic(fmt.Errorf("programmer error: ACRPullBinding.Spec.Auth has no method: %#v", spec.Auth)) | ||
| } | ||
| return credential.GetToken(ctx, policy.TokenRequestOptions{Scopes: []string{env.Services[cloud.ResourceManager].Audience + "/.default"}}) | ||
|
|
||
| // Default Audience to the Resource Manager audience for the environment | ||
| var audience string = env.Services[cloud.ResourceManager].Audience | ||
|
|
||
| if spec.Auth.FederatedIdentity != nil { | ||
| audience = "api://AzureADTokenExchange" | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Feel this should be switched based on the target environment. |
||
| } | ||
|
|
||
| return credential.GetToken(ctx, policy.TokenRequestOptions{Scopes: []string{audience + "/.default"}}) | ||
| } | ||
|
|
||
| func environment(input msiacrpullv1beta2.AzureEnvironmentType, config *msiacrpullv1beta2.AirgappedCloudConfiguration) cloud.Configuration { | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cygpath is only present in Cygwin installations. Shouldn't this be more portable in case its used in WSL etc?