-
Notifications
You must be signed in to change notification settings - Fork 188
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
[RFC-0007] Enable Azure OIDC for Azure DevOps repositories #1591
Conversation
f33d8af
to
030738f
Compare
96c2e90
to
8e52ad3
Compare
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.
I did some manual testing and everything looked good. I have left some comments in the docs for clarifications when creating the setup manually.
Following are some observations from GitRepository's perspective.
When a GitRepo is created without any provider specified and the default URL copied from the Azure DevOps web UI, the GitRepo fails with the following status:
status:
conditions:
- lastTransitionTime: "2024-09-12T14:37:24Z"
message: building artifact
observedGeneration: 1
reason: ProgressingWithRetry
status: "True"
type: Reconciling
- lastTransitionTime: "2024-09-12T14:37:24Z"
message: 'failed to checkout and determine revision: unable to clone ''https://[email protected]/my-test-org/fluxProjbelovedoleopard/_git/fluxRepobelovedoleopard'':
authentication required'
observedGeneration: 1
reason: GitOperationFailed
status: "False"
type: Ready
- lastTransitionTime: "2024-09-12T14:37:12Z"
message: 'failed to checkout and determine revision: unable to clone ''https://[email protected]/my-test-org/fluxProjbelovedoleopard/_git/fluxRepobelovedoleopard'':
authentication required'
observedGeneration: 1
reason: GitOperationFailed
status: "True"
type: FetchFailed
observedGeneration: -1
On setting .spec.provider
to azure
, it continues to fail with the same error, authentication required
. I don't think we can do anything about it as this is caused by the URL prefix. We can document about it. I'm not sure if we should internally adding custom URL handling to automatically remove the *@
prefix. I believe even for ssh addresses we ask users to make sure the address is properly created manually in our docs. This seems like a similar problem. I have left a inline suggestion to add this in the spec docs.
Once the URL is fixed by removing the initial *@
part of the URL, it just works:
status:
artifact:
digest: sha256:6ed79b079d9daf22c4dcca01a67711c40dbf0c950516631a4eb2747fb5617b2e
lastUpdateTime: "2024-09-12T14:57:02Z"
path: gitrepository/default/test-repo/7ab0c4af63272b6aa9038dbb7482ba2960d06a47.tar.gz
revision: main@sha1:7ab0c4af63272b6aa9038dbb7482ba2960d06a47
size: 141
url: http://source-controller.flux-system.svc.cluster.local./gitrepository/default/test-repo/7ab0c4
af63272b6aa9038dbb7482ba2960d06a47.tar.gz
conditions:
- lastTransitionTime: "2024-09-12T14:57:02Z"
message: stored artifact for revision 'main@sha1:7ab0c4af63272b6aa9038dbb7482ba2960d06a47'
observedGeneration: 1
reason: Succeeded
status: "True"
type: Ready
- lastTransitionTime: "2024-09-12T14:57:02Z"
message: stored artifact for revision 'main@sha1:7ab0c4af63272b6aa9038dbb7482ba2960d06a47'
observedGeneration: 1
reason: Succeeded
status: "True"
type: ArtifactInStorage
observedGeneration: 1
On removing the user permission to the specific project, it results in repository not found
error, which is expected, I believe.
status:
artifact:
digest: sha256:6ed79b079d9daf22c4dcca01a67711c40dbf0c950516631a4eb2747fb5617b2e
lastUpdateTime: "2024-09-12T15:01:52Z"
path: gitrepository/default/test-repo/7ab0c4af63272b6aa9038dbb7482ba2960d06a47.tar.gz
revision: main@sha1:7ab0c4af63272b6aa9038dbb7482ba2960d06a47
size: 141
url: http://source-controller.flux-system.svc.cluster.local./gitrepository/default/test-repo/7ab0c4af63272b6aa9038dbb7482ba2960d06a47.tar.gz
conditions:
- lastTransitionTime: "2024-09-12T15:03:28Z"
message: reconciliation in progress
observedGeneration: 2
reason: ProgressingWithRetry
status: "True"
type: Reconciling
- lastTransitionTime: "2024-09-12T15:03:28Z"
message: 'failed to checkout and determine revision: unable to list remote for
''https://dev.azure.com/my-test-org/fluxProjbelovedoleopard/_git/fluxRepobelovedoleopard'':
repository not found'
observedGeneration: 2
reason: GitOperationFailed
status: "False"
type: Ready
- lastTransitionTime: "2024-09-12T15:01:52Z"
message: stored artifact for revision 'main@sha1:7ab0c4af63272b6aa9038dbb7482ba2960d06a47'
observedGeneration: 2
reason: Succeeded
status: "True"
type: ArtifactInStorage
- lastTransitionTime: "2024-09-12T15:03:28Z"
message: 'failed to checkout and determine revision: unable to list remote for
''https://dev.azure.com/my-test-org/fluxProjbelovedoleopard/_git/fluxRepobelovedoleopard'':
repository not found'
observedGeneration: 2
reason: GitOperationFailed
status: "True"
type: FetchFailed
lastHandledReconcileAt: "2024-09-12T20:33:28.109759723+05:30"
observedGeneration: 2
When using an ssh address for URL, the following is observed:
status:
conditions:
- lastTransitionTime: "2024-09-12T16:13:53Z"
message: building artifact
observedGeneration: 2
reason: ProgressingWithRetry
status: "True"
type: Reconciling
- lastTransitionTime: "2024-09-12T16:13:53Z"
message: 'failed to configure authentication options: invalid ''ssh'' auth option:
''identity'' is required'
observedGeneration: 2
reason: AuthenticationFailed
status: "False"
type: Ready
- lastTransitionTime: "2024-09-12T16:13:29Z"
message: 'failed to configure authentication options: invalid ''ssh'' auth option:
''identity'' is required'
observedGeneration: 2
reason: AuthenticationFailed
status: "True"
type: FetchFailed
observedGeneration: 1
The error comes from AuthOptions.Validate()
in pkg/git as for ssh the secret data is expected to have an identity. I think the error is obvious enough and we don't need to add more validation to make sure when a provider is set, the URL is required to be an http/s address.
9f129ce
to
bd6ad3a
Compare
bd6ad3a
to
772131a
Compare
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.
LGTM!
The fluxcd/pkg dependencies can be updated in go.mod once those packages are available before merging this.
Thanks for the implementation and the detailed docs.
- Add a new provider field to GitRepository API spec which can be set to azure to enable passwordless authentication to Azure DevOps repositories. - API docs for new provider field and guidance to setup Azure environment with workload identity. - Controller changes to set the provider options in git authoptions to fetch credential while cloning the repository. - Add unit tests for testing provider Signed-off-by: Dipti Pai <[email protected]>
772131a
to
48417bd
Compare
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.
LGTM
Thanks @dipti-pai 🥇
from my side I see this error "repository not found" I've changed the provided to "azure" and have configured the needed stuff that mentioned in the document, also have removed the secretRef part, the URL looks as suggested, any idea why? |
"repository not found" error occurs when the permissions are not configured correctly. Could you check that the managed identity has access to the project that the Azure DevOps Repository is in ? |
I've tried to give the managed identity only contributor and then I tried to give it almost everything, I actually assigning these permissions to the managed identity of the AKS cluster (node agent), and I do this through azure devops project settings --> repo --> security |
the issue is resolved now, the managed identity takes "stakeholder" as the initial account type in azure devops I needed to upgrade it to basic |
This PR includes changes to enable oidc for Azure for git repositories. Changes include
generic
and can be set toazure
for enabling passwordless authentication for Azure.azidentity
APIs and fetch the access token for git repository.Closes: #1284