feat(azure-pipelines): add service principal authentication via reusable Azure provider - #7931
feat(azure-pipelines): add service principal authentication via reusable Azure provider#7931m-amaresh wants to merge 2 commits into
Conversation
|
Thank you for your contribution! 🙏 Please understand that we will do our best to review your PR and give you feedback as soon as possible, but please bear with us if it takes a little longer as expected. While you are waiting, make sure to:
Once the initial tests are successful, a KEDA member will ensure that the e2e tests are run. Once the e2e tests have been successfully completed, the PR may be merged at a later date. Please be patient. Learn more about our contribution guide. |
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
![semgrep-code-kedacore[bot]](https://avatars.githubusercontent.com/u/49917779?s=60&v=4){kind=small}
| logger := logr.Discard() | ||
|
|
||
| meta, _, err := parseAzurePipelinesMetadata(context.TODO(), logger, &scalersconfig.ScalerConfig{TriggerMetadata: metadata, ResolvedEnv: nil, AuthParams: authParams, TriggerIndex: testData.triggerIndex}, http.DefaultClient) | ||
| meta, err := parseAzurePipelinesMetadata(context.TODO(), logger, &scalersconfig.ScalerConfig{TriggerMetadata: metadata, ResolvedEnv: nil, AuthParams: authParams, TriggerIndex: testData.triggerIndex}, http.DefaultClient) |
There was a problem hiding this comment.
Consider to use well-defined context
🧼 Fixed in commit 6799601 🧼
| } | ||
| logger := logr.Discard() | ||
| _, _, err := parseAzurePipelinesMetadata(context.TODO(), logger, &scalersconfig.ScalerConfig{TriggerMetadata: testData.metadata, ResolvedEnv: nil, AuthParams: authParams}, http.DefaultClient) | ||
| _, err := parseAzurePipelinesMetadata(context.TODO(), logger, &scalersconfig.ScalerConfig{TriggerMetadata: testData.metadata, ResolvedEnv: nil, AuthParams: authParams}, http.DefaultClient) |
There was a problem hiding this comment.
Consider to use well-defined context
🧁 Fixed in commit 6799601 🧁
| logger := logr.Discard() | ||
|
|
||
| _, _, err := parseAzurePipelinesMetadata(context.TODO(), logger, &scalersconfig.ScalerConfig{TriggerMetadata: testData.metadata, ResolvedEnv: testData.resolvedEnv, AuthParams: testData.authParams}, http.DefaultClient) | ||
| _, err := parseAzurePipelinesMetadata(context.TODO(), logger, &scalersconfig.ScalerConfig{TriggerMetadata: testData.metadata, ResolvedEnv: testData.resolvedEnv, AuthParams: testData.authParams}, http.DefaultClient) |
There was a problem hiding this comment.
Consider to use well-defined context
🧼 Fixed in commit 6799601 🧼
m-amaresh
changed the title
m-amaresh
changed the title
|
This PR is now ready for review |
|
Hi @m-amaresh , Merge conflict. Would you consider adding resolvedEnv i.e. environment variables for service principal as supported by azidentity - https://github.com/Azure/azure-sdk-for-go/blob/main/sdk/azidentity/README.md#service-principal-with-secret |
|
@deepakbshetty I prefer to keep this PR scoped to adding reusable service principal auth and wiring it into the az devops scaler only. Also those env vars come from the process env which here is the operator pod that means one service principal for the entire cluster. |
…ble Azure auth provider Introduce a reusable `azureServicePrincipal` authentication provider on TriggerAuthentication/ClusterTriggerAuthentication and wire it into the Azure Pipelines scaler. Per maintainer feedback on kedacore#7578, the provider lives in pkg/scalers/azure so other Azure scalers can adopt it, rather than being scoped to the Azure Pipelines scaler alone. - Add AzureServicePrincipal spec (tenantId, clientId, clientSecret, clientCertificate, clientCertificatePassword, cloud, activeDirectoryEndpoint) with webhook validation and generated CRDs - Add azure.NewServicePrincipalCredential supporting client secret and PKCS#12/PEM certificate credentials, sovereign clouds, and private cloud (with instance discovery disabled) - Resolve provider secrets in scale_resolvers into azureServicePrincipal* auth params - Refactor Azure Pipelines auth into configureAzurePipelinesAuth with an explicit auth type; preserve existing PAT > workload identity precedence and add service principal as a fallback - Add unit tests and an e2e test (tests/scalers/azure/azure_pipelines_sp) Signed-off-by: m-amaresh <115941749+m-amaresh@users.noreply.github.com>
…tently Signed-off-by: m-amaresh <115941749+m-amaresh@users.noreply.github.com>
|
Hi @m-amaresh That's fine if you want to keep you PR scope as-is. The environment variables on triggerauth are retrieved from individual deployment see existing personalAccessTokenFromEnv and not from keda operator as per my understanding. |
2 participants
Provide a description of what has been changed
Adds Microsoft Entra service principal authentication to the Azure Pipelines scaler.
Following maintainer feedback on #7578, the credential handling is implemented as a
reusable
azureServicePrincipalauthentication provider onTriggerAuthenticationand
ClusterTriggerAuthentication, living inpkg/scalers/azure, instead of beinglocal to the Azure Pipelines scaler. Other Azure scalers can adopt it in follow-ups
via
azure.IsServicePrincipalAuthConfigured/azure.NewServicePrincipalCredential.What's included
AzureServicePrincipalTriggerAuthentication spec:tenantId,clientId,clientSecret,clientCertificate,clientCertificatePassword,cloud,activeDirectoryEndpoint, with admission-webhook validation and regenerated CRDs.azure.NewServicePrincipalCredential: client-secret and certificate (PKCS#12 / PEM)credentials, sovereign clouds (US Gov, China), and
Privatecloud support(instance discovery disabled).
pkg/scaling/resolverto read the referenced Secrets.configureAzurePipelinesAuthwith an explicitauth type. Existing precedence is preserved — PAT wins, then Azure AD Workload
Identity — with service principal added as a fallback when neither is configured.
Reviewer notes
tests/scalers/azure/azure_pipelines_sp) needs the shared CIservice principal (
TF_AZURE_SP_APP_ID/AZURE_SP_KEY/TF_AZURE_SP_TENANT)added to the Azure DevOps organization with read access to the agent pool and its
job requests. This is a kedacore infra prerequisite — happy to coordinate.
Checklist
make generate-scalers-schemahas been run to update any outdated generated filesFixes #4853
Relates to #7581
Relates to #7578
Relates to kedacore/keda-docs#1814
Relates to kedacore/charts#888