diff --git a/CHANGELOG.md b/CHANGELOG.md index 06a59e6ae9d..901bc29e4a2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -79,6 +79,7 @@ To learn more about active deprecations, we recommend checking [GitHub Discussio - **General**: Add CRD-level validation markers (Minimum, MinLength, MinItems, Enum) for ScaledObject, ScaledJob, ScaleTriggers, and TriggerAuthentication API types ([#7533](https://github.com/kedacore/keda/pull/7533)) - **General**: Add `--leader-election-id` flag to allow configuring the leader election Lease name ([#7564](https://github.com/kedacore/keda/issues/7564)) - **General**: Make APIService cert injections optional ([#7559](https://github.com/kedacore/keda/pull/7559)) +- **Azure Pipelines Scaler**: Add service principal authentication support with client secret and client certificate ([#4853](https://github.com/kedacore/keda/issues/4853)) - **Elasticsearch Scaler**: Add HTTP status check for Elasticsearch errors ([#7480](https://github.com/kedacore/keda/pull/7480)) - **Kubernetes Workload Scaler**: Add `groupByNode` parameter ([#7628](https://github.com/kedacore/keda/issues/7628)) diff --git a/pkg/scalers/azure_pipelines_scaler.go b/pkg/scalers/azure_pipelines_scaler.go index 1a17a2e4dde..b6ed3bf38f6 100644 --- a/pkg/scalers/azure_pipelines_scaler.go +++ b/pkg/scalers/azure_pipelines_scaler.go @@ -126,17 +126,21 @@ type azurePipelinesPoolIDResponse struct { } type azurePipelinesScaler struct { - metricType v2.MetricTargetType - metadata *azurePipelinesMetadata - httpClient *http.Client - podIdentity kedav1alpha1.AuthPodIdentity - logger logr.Logger + metricType v2.MetricTargetType + metadata *azurePipelinesMetadata + httpClient *http.Client + logger logr.Logger } type azurePipelinesMetadata struct { OrganizationURL string `keda:"name=organizationURL, order=resolvedEnv;authParams;triggerMetadata"` OrganizationName string PersonalAccessToken string `keda:"name=personalAccessToken, order=authParams;resolvedEnv, optional"` + TenantID string `keda:"name=tenantId, order=authParams;triggerMetadata;resolvedEnv, optional"` + ClientID string `keda:"name=clientId, order=authParams;triggerMetadata;resolvedEnv, optional"` + ClientSecret string `keda:"name=clientSecret, order=authParams;resolvedEnv, optional"` + ClientCertificate string `keda:"name=clientCertificate, order=authParams;resolvedEnv, optional"` + ClientCertificatePassword string `keda:"name=clientCertificatePassword, order=authParams;resolvedEnv, optional"` authContext authContext Parent string `keda:"name=parent, order=triggerMetadata, optional"` Demands string `keda:"name=demands, order=triggerMetadata, optional"` @@ -153,9 +157,30 @@ type azurePipelinesMetadata struct { } type authContext struct { - cred *azidentity.ChainedTokenCredential - pat string - token *azcore.AccessToken + cred azcore.TokenCredential + pat string + token *azcore.AccessToken + authType azurePipelinesAuthType +} + +type azurePipelinesAuthType string + +const ( + azurePipelinesAuthTypePAT azurePipelinesAuthType = "pat" + azurePipelinesAuthTypeServicePrincipal azurePipelinesAuthType = "service-principal" + azurePipelinesAuthTypeWorkloadIdentity azurePipelinesAuthType = "workload-identity" +) + +var newAzurePipelinesClientSecretCredential = func(tenantID, clientID, clientSecret string) (azcore.TokenCredential, error) { + return azidentity.NewClientSecretCredential(tenantID, clientID, clientSecret, nil) +} + +var newAzurePipelinesClientCertificateCredential = func(tenantID, clientID, clientCertificate, clientCertificatePassword string) (azcore.TokenCredential, error) { + certs, key, err := azidentity.ParseCertificates([]byte(clientCertificate), []byte(clientCertificatePassword)) + if err != nil { + return nil, err + } + return azidentity.NewClientCertificateCredential(tenantID, clientID, certs, key, nil) } func (a *azurePipelinesMetadata) Validate() error { @@ -177,84 +202,124 @@ func NewAzurePipelinesScaler(ctx context.Context, config *scalersconfig.ScalerCo return nil, fmt.Errorf("error getting scaler metric type: %w", err) } - meta, podIdentity, err := parseAzurePipelinesMetadata(ctx, logger, config, httpClient) + meta, err := parseAzurePipelinesMetadata(ctx, logger, config, httpClient) if err != nil { return nil, fmt.Errorf("error parsing azure Pipelines metadata: %w", err) } return &azurePipelinesScaler{ - metricType: metricType, - metadata: meta, - httpClient: httpClient, - podIdentity: podIdentity, - logger: logger, + metricType: metricType, + metadata: meta, + httpClient: httpClient, + logger: logger, }, nil } -func getAuthMethod(logger logr.Logger, config *scalersconfig.ScalerConfig, meta *azurePipelinesMetadata) (*azidentity.ChainedTokenCredential, kedav1alpha1.AuthPodIdentity, error) { - if meta.PersonalAccessToken != "" { - meta.authContext.pat = strings.TrimSuffix(meta.PersonalAccessToken, "\n") - return nil, kedav1alpha1.AuthPodIdentity{}, nil - } - +func getAuthMethod(logger logr.Logger, config *scalersconfig.ScalerConfig, meta *azurePipelinesMetadata) (azcore.TokenCredential, error) { switch config.PodIdentity.Provider { case "", kedav1alpha1.PodIdentityProviderNone: - return nil, kedav1alpha1.AuthPodIdentity{}, fmt.Errorf("no personalAccessToken given or PodIdentity provider configured") + if meta.PersonalAccessToken != "" { + meta.authContext.pat = strings.TrimSuffix(meta.PersonalAccessToken, "\n") + meta.authContext.authType = azurePipelinesAuthTypePAT + return nil, nil + } + + if meta.TenantID != "" || meta.ClientID != "" || meta.ClientSecret != "" || meta.ClientCertificate != "" || meta.ClientCertificatePassword != "" { + missing := make([]string, 0, 3) + if meta.TenantID == "" { + missing = append(missing, "tenantId") + } + if meta.ClientID == "" { + missing = append(missing, "clientId") + } + usingClientSecret := meta.ClientSecret != "" + usingClientCertificate := meta.ClientCertificate != "" + if usingClientSecret && usingClientCertificate { + return nil, fmt.Errorf("clientSecret and clientCertificate are mutually exclusive") + } + if !usingClientSecret && !usingClientCertificate { + missing = append(missing, "one of clientSecret or clientCertificate") + } + if meta.ClientCertificatePassword != "" && !usingClientCertificate { + return nil, fmt.Errorf("clientCertificatePassword requires clientCertificate") + } + if len(missing) != 0 { + return nil, fmt.Errorf("incomplete service principal configuration, missing %s", strings.Join(missing, ", ")) + } + + var ( + cred azcore.TokenCredential + err error + ) + if usingClientCertificate { + cred, err = newAzurePipelinesClientCertificateCredential(meta.TenantID, meta.ClientID, meta.ClientCertificate, meta.ClientCertificatePassword) + } else { + cred, err = newAzurePipelinesClientSecretCredential(meta.TenantID, meta.ClientID, meta.ClientSecret) + } + if err != nil { + return nil, err + } + meta.authContext.authType = azurePipelinesAuthTypeServicePrincipal + return cred, nil + } + + return nil, fmt.Errorf("no personalAccessToken, service principal credentials, or PodIdentity provider configured") case kedav1alpha1.PodIdentityProviderAzureWorkload: cred, err := azure.NewChainedCredential(logger, config.PodIdentity) if err != nil { - return nil, kedav1alpha1.AuthPodIdentity{}, err + return nil, err } - return cred, kedav1alpha1.AuthPodIdentity{Provider: config.PodIdentity.Provider}, nil + meta.authContext.authType = azurePipelinesAuthTypeWorkloadIdentity + return cred, nil default: - return nil, kedav1alpha1.AuthPodIdentity{}, fmt.Errorf("pod identity %s not supported for azure pipelines", config.PodIdentity.Provider) + return nil, fmt.Errorf("pod identity %s not supported for azure pipelines", config.PodIdentity.Provider) } } -func parseAzurePipelinesMetadata(ctx context.Context, logger logr.Logger, config *scalersconfig.ScalerConfig, httpClient *http.Client) (*azurePipelinesMetadata, kedav1alpha1.AuthPodIdentity, error) { +func parseAzurePipelinesMetadata(ctx context.Context, logger logr.Logger, config *scalersconfig.ScalerConfig, httpClient *http.Client) (*azurePipelinesMetadata, error) { if config.TriggerMetadata["jobsToFetch"] != "" && config.TriggerMetadata["fetchUnfinishedJobsOnly"] != "" { - return nil, kedav1alpha1.AuthPodIdentity{}, fmt.Errorf("cannot specify both jobsToFetch and fetchUnfinishedJobsOnly at the same time") + return nil, fmt.Errorf("cannot specify both jobsToFetch and fetchUnfinishedJobsOnly at the same time") } if config.TriggerMetadata["jobsToFetch"] != "" && config.TriggerMetadata["parent"] != "" { - return nil, kedav1alpha1.AuthPodIdentity{}, fmt.Errorf("cannot specify both jobsToFetch and parent at the same time") + return nil, fmt.Errorf("cannot specify both jobsToFetch and parent at the same time") } meta := &azurePipelinesMetadata{} if err := config.TypedConfig(meta); err != nil { - return nil, kedav1alpha1.AuthPodIdentity{}, fmt.Errorf("error parsing azure pipeline metadata: %w", err) + return nil, fmt.Errorf("error parsing azure pipeline metadata: %w", err) } meta.triggerIndex = config.TriggerIndex - cred, podIdentity, err := getAuthMethod(logger, config, meta) + cred, err := getAuthMethod(logger, config, meta) if err != nil { - return nil, kedav1alpha1.AuthPodIdentity{}, err + return nil, err } meta.authContext.cred = cred meta.authContext.token = nil if meta.PoolName != "" { - poolID, err := getPoolIDFromName(ctx, logger, meta.PoolName, meta, podIdentity, httpClient) + poolID, err := getPoolIDFromName(ctx, logger, meta.PoolName, meta, httpClient) if err != nil { - return nil, kedav1alpha1.AuthPodIdentity{}, err + return nil, err } meta.PoolID = poolID } else if meta.PoolID != 0 { - _, err := validatePoolID(ctx, logger, meta.PoolID, meta, podIdentity, httpClient) + _, err := validatePoolID(ctx, logger, meta.PoolID, meta, httpClient) if err != nil { - return nil, kedav1alpha1.AuthPodIdentity{}, err + return nil, err } } else { - return nil, kedav1alpha1.AuthPodIdentity{}, fmt.Errorf("no poolName or poolID given") + return nil, fmt.Errorf("no poolName or poolID given") } - return meta, podIdentity, nil + return meta, nil } -func getPoolIDFromName(ctx context.Context, logger logr.Logger, poolName string, metadata *azurePipelinesMetadata, podIdentity kedav1alpha1.AuthPodIdentity, httpClient *http.Client) (int, error) { +func getPoolIDFromName(ctx context.Context, logger logr.Logger, poolName string, metadata *azurePipelinesMetadata, httpClient *http.Client) (int, error) { urlString := fmt.Sprintf("%s/_apis/distributedtask/pools?poolName=%s", metadata.OrganizationURL, url.QueryEscape(poolName)) - body, err := getAzurePipelineRequest(ctx, logger, urlString, metadata, podIdentity, httpClient) + body, err := getAzurePipelineRequest(ctx, logger, urlString, metadata, httpClient) if err != nil { return -1, err @@ -278,9 +343,9 @@ func getPoolIDFromName(ctx context.Context, logger logr.Logger, poolName string, return result.Value[0].ID, nil } -func validatePoolID(ctx context.Context, logger logr.Logger, poolID int, metadata *azurePipelinesMetadata, podIdentity kedav1alpha1.AuthPodIdentity, httpClient *http.Client) (int, error) { +func validatePoolID(ctx context.Context, logger logr.Logger, poolID int, metadata *azurePipelinesMetadata, httpClient *http.Client) (int, error) { urlString := fmt.Sprintf("%s/_apis/distributedtask/pools?poolID=%d", metadata.OrganizationURL, poolID) - body, err := getAzurePipelineRequest(ctx, logger, urlString, metadata, podIdentity, httpClient) + body, err := getAzurePipelineRequest(ctx, logger, urlString, metadata, httpClient) if err != nil { return -1, fmt.Errorf("agent pool with id `%d` not found: %w", poolID, err) @@ -317,26 +382,32 @@ func getToken(ctx context.Context, metadata *azurePipelinesMetadata, scope strin return metadata.authContext.token.Token, nil } -func getAzurePipelineRequest(ctx context.Context, logger logr.Logger, urlString string, metadata *azurePipelinesMetadata, podIdentity kedav1alpha1.AuthPodIdentity, httpClient *http.Client) ([]byte, error) { +func getAzurePipelineRequest(ctx context.Context, logger logr.Logger, urlString string, metadata *azurePipelinesMetadata, httpClient *http.Client) ([]byte, error) { req, err := http.NewRequestWithContext(ctx, "GET", urlString, nil) if err != nil { return []byte{}, err } - switch podIdentity.Provider { - case "", kedav1alpha1.PodIdentityProviderNone: - //PAT + authType := metadata.authContext.authType + + switch authType { + case azurePipelinesAuthTypePAT: logger.V(1).Info("making request to ADO REST API using PAT") req.SetBasicAuth("", metadata.authContext.pat) - case kedav1alpha1.PodIdentityProviderAzureWorkload: - //ADO Resource token - logger.V(1).Info("making request to ADO REST API using managed identity") + case azurePipelinesAuthTypeServicePrincipal, azurePipelinesAuthTypeWorkloadIdentity: + authMethod := "service principal" + if authType == azurePipelinesAuthTypeWorkloadIdentity { + authMethod = "workload identity" + } + logger.V(1).Info("making request to ADO REST API using bearer token", "authMethod", authMethod) aadToken, err := getToken(ctx, metadata, devopsResource) if err != nil { - return []byte{}, fmt.Errorf("cannot create workload identity credentials: %w", err) + return []byte{}, fmt.Errorf("cannot acquire %s token: %w", authMethod, err) } logger.V(1).Info("token acquired setting auth header as 'bearer XXXXXX'") req.Header.Set("Authorization", fmt.Sprintf("Bearer %s", aadToken)) + default: + return []byte{}, fmt.Errorf("no authentication method configured for azure pipelines") } r, err := httpClient.Do(req) @@ -390,7 +461,7 @@ func (s *azurePipelinesScaler) GetAzurePipelinesQueueLength(ctx context.Context) return -1, err } - body, err := getAzurePipelineRequest(ctx, s.logger, urlString, s.metadata, s.podIdentity, s.httpClient) + body, err := getAzurePipelineRequest(ctx, s.logger, urlString, s.metadata, s.httpClient) if err != nil { return -1, err } diff --git a/pkg/scalers/azure_pipelines_scaler_test.go b/pkg/scalers/azure_pipelines_scaler_test.go index a623d8cc0d7..fb7c1d9f7e1 100644 --- a/pkg/scalers/azure_pipelines_scaler_test.go +++ b/pkg/scalers/azure_pipelines_scaler_test.go @@ -8,6 +8,8 @@ import ( "strings" "testing" + "github.com/Azure/azure-sdk-for-go/sdk/azcore" + "github.com/Azure/azure-sdk-for-go/sdk/azcore/policy" "github.com/go-logr/logr" "github.com/kedacore/keda/v2/pkg/scalers/scalersconfig" @@ -37,10 +39,20 @@ var testAzurePipelinesMetadata = []parseAzurePipelinesMetadataTestData{ {"using triggerAuthentication", map[string]string{"poolID": "1", "targetPipelinesQueueLength": "1"}, false, testAzurePipelinesResolvedEnv, map[string]string{"organizationURL": "https://dev.azure.com/sample", "personalAccessToken": "sample"}}, // using triggerAuthentication with personalAccessToken terminating in newline {"using triggerAuthentication with personalAccessToken terminating in newline", map[string]string{"poolID": "1", "targetPipelinesQueueLength": "1"}, false, testAzurePipelinesResolvedEnv, map[string]string{"organizationURL": "https://dev.azure.com/sample", "personalAccessToken": "sample\n"}}, + // using service principal authentication + {"using service principal authentication", map[string]string{"poolID": "1", "targetPipelinesQueueLength": "1"}, false, testAzurePipelinesResolvedEnv, map[string]string{"organizationURL": "https://dev.azure.com/sample", "tenantId": "tenant", "clientId": "client", "clientSecret": "secret"}}, + // using service principal certificate authentication + {"using service principal certificate authentication", map[string]string{"poolID": "1", "targetPipelinesQueueLength": "1"}, false, testAzurePipelinesResolvedEnv, map[string]string{"organizationURL": "https://dev.azure.com/sample", "tenantId": "tenant", "clientId": "client", "clientCertificate": "cert-data", "clientCertificatePassword": "cert-password"}}, // missing organizationURL {"missing organizationURL", map[string]string{"organizationURLFromEnv": "", "personalAccessTokenFromEnv": "sample", "poolID": "1", "targetPipelinesQueueLength": "1"}, true, testAzurePipelinesResolvedEnv, map[string]string{}}, - // missing personalAccessToken - {"missing personalAccessToken", map[string]string{"organizationURLFromEnv": "AZP_URL", "poolID": "1", "targetPipelinesQueueLength": "1"}, true, testAzurePipelinesResolvedEnv, map[string]string{}}, + // missing all auth methods + {"missing all auth methods", map[string]string{"organizationURLFromEnv": "AZP_URL", "poolID": "1", "targetPipelinesQueueLength": "1"}, true, testAzurePipelinesResolvedEnv, map[string]string{}}, + // incomplete service principal + {"incomplete service principal", map[string]string{"poolID": "1", "targetPipelinesQueueLength": "1"}, true, testAzurePipelinesResolvedEnv, map[string]string{"organizationURL": "https://dev.azure.com/sample", "tenantId": "tenant", "clientId": "client"}}, + // mutually exclusive secret and certificate + {"mutually exclusive service principal auth", map[string]string{"poolID": "1", "targetPipelinesQueueLength": "1"}, true, testAzurePipelinesResolvedEnv, map[string]string{"organizationURL": "https://dev.azure.com/sample", "tenantId": "tenant", "clientId": "client", "clientSecret": "secret", "clientCertificate": "cert-data"}}, + // certificate password without certificate + {"certificate password without certificate", map[string]string{"poolID": "1", "targetPipelinesQueueLength": "1"}, true, testAzurePipelinesResolvedEnv, map[string]string{"organizationURL": "https://dev.azure.com/sample", "tenantId": "tenant", "clientId": "client", "clientCertificatePassword": "cert-password"}}, // missing poolID {"missing poolID", map[string]string{"organizationURLFromEnv": "AZP_URL", "personalAccessTokenFromEnv": "AZP_TOKEN", "poolID": "", "targetPipelinesQueueLength": "1"}, true, testAzurePipelinesResolvedEnv, map[string]string{}}, // activationTargetPipelinesQueueLength malformed @@ -64,13 +76,25 @@ func TestParseAzurePipelinesMetadata(t *testing.T) { for _, testData := range testAzurePipelinesMetadata { t.Run(testData.testName, func(t *testing.T) { var apiStub = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - personalAccessToken := strings.Split(r.Header["Authorization"][0], " ")[1] - if personalAccessToken != "" && personalAccessToken[len(personalAccessToken)-1:] == "\n" { + authHeader := r.Header.Get("Authorization") + switch { + case strings.HasPrefix(authHeader, "Basic "): + personalAccessToken := strings.Split(authHeader, " ")[1] + if personalAccessToken != "" && personalAccessToken[len(personalAccessToken)-1:] == "\n" { + w.WriteHeader(http.StatusUnauthorized) + return + } + case strings.HasPrefix(authHeader, "Bearer "): + if authHeader != "Bearer fake-token" { + w.WriteHeader(http.StatusUnauthorized) + return + } + default: w.WriteHeader(http.StatusUnauthorized) - } else { - w.WriteHeader(http.StatusOK) - _, _ = w.Write([]byte(`{"count":1,"value":[{"id":1}]}`)) + return } + w.WriteHeader(http.StatusOK) + _, _ = w.Write([]byte(`{"count":1,"value":[{"id":1}]}`)) })) // set urls into local stub only if they are already defined @@ -82,8 +106,26 @@ func TestParseAzurePipelinesMetadata(t *testing.T) { } logger := logr.Discard() + if testData.authParams["clientSecret"] != "" { + origNewClientSecretCredential := newAzurePipelinesClientSecretCredential + newAzurePipelinesClientSecretCredential = func(_, _, _ string) (azcore.TokenCredential, error) { + return fakeTokenCredential{token: "fake-token"}, nil + } + defer func() { + newAzurePipelinesClientSecretCredential = origNewClientSecretCredential + }() + } + if testData.authParams["clientCertificate"] != "" { + origNewClientCertificateCredential := newAzurePipelinesClientCertificateCredential + newAzurePipelinesClientCertificateCredential = func(_, _, _, _ string) (azcore.TokenCredential, error) { + return fakeTokenCredential{token: "fake-token"}, nil + } + defer func() { + newAzurePipelinesClientCertificateCredential = origNewClientCertificateCredential + }() + } - _, _, err := parseAzurePipelinesMetadata(context.TODO(), logger, &scalersconfig.ScalerConfig{TriggerMetadata: testData.metadata, ResolvedEnv: testData.resolvedEnv, AuthParams: testData.authParams}, http.DefaultClient) + _, err := parseAzurePipelinesMetadata(context.Background(), logger, &scalersconfig.ScalerConfig{TriggerMetadata: testData.metadata, ResolvedEnv: testData.resolvedEnv, AuthParams: testData.authParams}, http.DefaultClient) if err != nil && !testData.isError { t.Error("Expected success but got error", err) } @@ -94,6 +136,14 @@ func TestParseAzurePipelinesMetadata(t *testing.T) { } } +type fakeTokenCredential struct { + token string +} + +func (f fakeTokenCredential) GetToken(context.Context, policy.TokenRequestOptions) (azcore.AccessToken, error) { + return azcore.AccessToken{Token: f.token}, nil +} + type validateAzurePipelinesPoolTestData struct { testName string metadata map[string]string @@ -137,7 +187,7 @@ func TestValidateAzurePipelinesPool(t *testing.T) { "personalAccessToken": "PAT", } logger := logr.Discard() - _, _, err := parseAzurePipelinesMetadata(context.TODO(), logger, &scalersconfig.ScalerConfig{TriggerMetadata: testData.metadata, ResolvedEnv: nil, AuthParams: authParams}, http.DefaultClient) + _, err := parseAzurePipelinesMetadata(context.Background(), logger, &scalersconfig.ScalerConfig{TriggerMetadata: testData.metadata, ResolvedEnv: nil, AuthParams: authParams}, http.DefaultClient) if err != nil && !testData.isError { t.Error("Expected success but got error", err) } @@ -177,7 +227,7 @@ func TestAzurePipelinesGetMetricSpecForScaling(t *testing.T) { 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.Background(), logger, &scalersconfig.ScalerConfig{TriggerMetadata: metadata, ResolvedEnv: nil, AuthParams: authParams, TriggerIndex: testData.triggerIndex}, http.DefaultClient) if err != nil { t.Fatal("Could not parse metadata:", err) } @@ -201,6 +251,7 @@ func getMatchedAgentMetaData(url string) *azurePipelinesMetadata { meta.OrganizationURL = url meta.Parent = "dotnet60-keda-template" meta.authContext.pat = "testPAT" + meta.authContext.authType = azurePipelinesAuthTypePAT meta.PoolID = 1 meta.TargetPipelinesQueueLength = 1 @@ -220,7 +271,7 @@ func TestAzurePipelinesMatchedAgent(t *testing.T) { httpClient: http.DefaultClient, } - queueLen, err := mockAzurePipelinesScaler.GetAzurePipelinesQueueLength(context.TODO()) + queueLen, err := mockAzurePipelinesScaler.GetAzurePipelinesQueueLength(context.Background()) if err != nil { t.Fail() @@ -296,7 +347,7 @@ func TestAzurePipelinesMatchedDemandAgent(t *testing.T) { httpClient: http.DefaultClient, } - queueLen, err := mockAzurePipelinesScaler.GetAzurePipelinesQueueLength(context.TODO()) + queueLen, err := mockAzurePipelinesScaler.GetAzurePipelinesQueueLength(context.Background()) if err != nil { t.Fail() @@ -321,7 +372,7 @@ func TestAzurePipelinesNonMatchedDemandAgent(t *testing.T) { httpClient: http.DefaultClient, } - queueLen, err := mockAzurePipelinesScaler.GetAzurePipelinesQueueLength(context.TODO()) + queueLen, err := mockAzurePipelinesScaler.GetAzurePipelinesQueueLength(context.Background()) if err != nil { t.Fail() @@ -346,7 +397,7 @@ func TestAzurePipelinesMatchedDemandAgentWithRequireAllDemands(t *testing.T) { httpClient: http.DefaultClient, } - queuelen, err := mockAzurePipelinesScaler.GetAzurePipelinesQueueLength(context.TODO()) + queuelen, err := mockAzurePipelinesScaler.GetAzurePipelinesQueueLength(context.Background()) if err != nil { t.Fail() @@ -372,8 +423,7 @@ func TestAzurePipelinesMatchedDemandAgentWithRequireAllDemandsAndIgnoreOthers(t httpClient: http.DefaultClient, } - // nosemgrep: context-todo - queuelen, err := mockAzurePipelinesScaler.GetAzurePipelinesQueueLength(context.TODO()) + queuelen, err := mockAzurePipelinesScaler.GetAzurePipelinesQueueLength(context.Background()) if err != nil { t.Fail() @@ -400,8 +450,7 @@ func TestAzurePipelinesNotMatchedPartialRequiredTriggerDemands(t *testing.T) { httpClient: http.DefaultClient, } - // nosemgrep: context-todo - queuelen, err := mockAzurePipelinesScaler.GetAzurePipelinesQueueLength(context.TODO()) + queuelen, err := mockAzurePipelinesScaler.GetAzurePipelinesQueueLength(context.Background()) if err != nil { t.Fail() @@ -505,6 +554,7 @@ func TestAzurePipelinesQueueURLTest(t *testing.T) { meta.OrganizationName = "testOrg" meta.OrganizationURL = apiStub.URL meta.authContext.pat = "testPAT" + meta.authContext.authType = azurePipelinesAuthTypePAT meta.PoolID = 1 meta.TargetPipelinesQueueLength = 1 @@ -526,3 +576,27 @@ func TestAzurePipelinesQueueURLTest(t *testing.T) { }) } } + +func TestAzurePipelinesRequestUsesBearerTokenForServicePrincipal(t *testing.T) { + apiStub := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if got := r.Header.Get("Authorization"); got != "Bearer fake-token" { + t.Fatalf("expected bearer token auth, got %q", got) + } + w.WriteHeader(http.StatusOK) + _, _ = w.Write([]byte(`{"count":1,"value":[{"id":1}]}`)) + })) + defer apiStub.Close() + + meta := &azurePipelinesMetadata{ + OrganizationURL: apiStub.URL, + authContext: authContext{ + cred: fakeTokenCredential{token: "fake-token"}, + authType: azurePipelinesAuthTypeServicePrincipal, + }, + } + + _, err := getAzurePipelineRequest(context.Background(), logr.Discard(), apiStub.URL, meta, http.DefaultClient) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } +} diff --git a/schema/generated/scalers-schema.json b/schema/generated/scalers-schema.json index da35bace415..ad70cf9c262 100644 --- a/schema/generated/scalers-schema.json +++ b/schema/generated/scalers-schema.json @@ -1102,6 +1102,43 @@ "envVariableReadable": true, "triggerAuthenticationVariableReadable": true }, + { + "name": "tenantId", + "type": "string", + "optional": true, + "metadataVariableReadable": true, + "envVariableReadable": true, + "triggerAuthenticationVariableReadable": true + }, + { + "name": "clientId", + "type": "string", + "optional": true, + "metadataVariableReadable": true, + "envVariableReadable": true, + "triggerAuthenticationVariableReadable": true + }, + { + "name": "clientSecret", + "type": "string", + "optional": true, + "envVariableReadable": true, + "triggerAuthenticationVariableReadable": true + }, + { + "name": "clientCertificate", + "type": "string", + "optional": true, + "envVariableReadable": true, + "triggerAuthenticationVariableReadable": true + }, + { + "name": "clientCertificatePassword", + "type": "string", + "optional": true, + "envVariableReadable": true, + "triggerAuthenticationVariableReadable": true + }, { "name": "parent", "type": "string", diff --git a/schema/generated/scalers-schema.yaml b/schema/generated/scalers-schema.yaml index 27dbf81d998..d28a1f985df 100644 --- a/schema/generated/scalers-schema.yaml +++ b/schema/generated/scalers-schema.yaml @@ -720,6 +720,33 @@ scalers: optional: true envVariableReadable: true triggerAuthenticationVariableReadable: true + - name: tenantId + type: string + optional: true + metadataVariableReadable: true + envVariableReadable: true + triggerAuthenticationVariableReadable: true + - name: clientId + type: string + optional: true + metadataVariableReadable: true + envVariableReadable: true + triggerAuthenticationVariableReadable: true + - name: clientSecret + type: string + optional: true + envVariableReadable: true + triggerAuthenticationVariableReadable: true + - name: clientCertificate + type: string + optional: true + envVariableReadable: true + triggerAuthenticationVariableReadable: true + - name: clientCertificatePassword + type: string + optional: true + envVariableReadable: true + triggerAuthenticationVariableReadable: true - name: parent type: string optional: true diff --git a/tests/scalers/azure/azure_pipelines_sp/azure_pipelines_sp_test.go b/tests/scalers/azure/azure_pipelines_sp/azure_pipelines_sp_test.go new file mode 100644 index 00000000000..d18ca6d7e7b --- /dev/null +++ b/tests/scalers/azure/azure_pipelines_sp/azure_pipelines_sp_test.go @@ -0,0 +1,354 @@ +//go:build e2e +// +build e2e + +package azure_pipelines_sp_test + +import ( + "context" + "encoding/base64" + "fmt" + "os" + "strconv" + "testing" + "time" + + "github.com/joho/godotenv" + "github.com/microsoft/azure-devops-go-api/azuredevops" + "github.com/microsoft/azure-devops-go-api/azuredevops/build" + "github.com/microsoft/azure-devops-go-api/azuredevops/taskagent" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "k8s.io/client-go/kubernetes" + + . "github.com/kedacore/keda/v2/tests/helper" +) + +// Load environment variables from .env file +var _ = godotenv.Load("../../../.env") + +const ( + testName = "azure-pipelines-sp-test" +) + +var ( + organizationURL = os.Getenv("AZURE_DEVOPS_ORGANIZATION_URL") + personalAccessToken = os.Getenv("AZURE_DEVOPS_PAT") + project = os.Getenv("AZURE_DEVOPS_PROJECT") + buildID = os.Getenv("AZURE_DEVOPS_BUILD_DEFINITION_ID") + poolName = os.Getenv("AZURE_DEVOPS_POOL_NAME") + azureADClientID = os.Getenv("TF_AZURE_SP_APP_ID") + azureADSecret = os.Getenv("AZURE_SP_KEY") + azureADTenantID = os.Getenv("TF_AZURE_SP_TENANT") + poolID = "0" + testNamespace = fmt.Sprintf("%s-ns", testName) + agentSecretName = fmt.Sprintf("%s-agent-secret", testName) + scalerSecretName = fmt.Sprintf("%s-scaler-secret", testName) + triggerAuthName = fmt.Sprintf("%s-ta", testName) + deploymentName = fmt.Sprintf("%s-deployment", testName) + scaledObjectName = fmt.Sprintf("%s-so", testName) + minReplicaCount = 0 + maxReplicaCount = 1 +) + +type templateData struct { + TestNamespace string + AgentSecretName string + ScalerSecretName string + TriggerAuthName string + DeploymentName string + ScaledObjectName string + MinReplicaCount string + MaxReplicaCount string + Pat string + URL string + PoolName string + PoolID string + AzureADClientID string + AzureADSecret string + AzureADTenantID string +} + +const ( + agentSecretTemplate = ` +apiVersion: v1 +kind: Secret +metadata: + name: {{.AgentSecretName}} + namespace: {{.TestNamespace}} +data: + personalAccessToken: {{.Pat}} +` + + scalerSecretTemplate = ` +apiVersion: v1 +kind: Secret +metadata: + name: {{.ScalerSecretName}} + namespace: {{.TestNamespace}} +data: + clientSecret: {{.AzureADSecret}} +` + + deploymentTemplate = ` +apiVersion: apps/v1 +kind: Deployment +metadata: + name: {{.DeploymentName}} + namespace: {{.TestNamespace}} + labels: + app: azdevops-agent +spec: + replicas: 1 + selector: + matchLabels: + app: azdevops-agent + template: + metadata: + labels: + app: azdevops-agent + spec: + terminationGracePeriodSeconds: 90 + containers: + - name: azdevops-agent + lifecycle: + preStop: + exec: + command: ["/bin/sleep","60"] + image: ghcr.io/kedacore/tests-azure-pipelines-agent:b3a02cc + env: + - name: AZP_URL + value: {{.URL}} + - name: AZP_TOKEN + valueFrom: + secretKeyRef: + name: {{.AgentSecretName}} + key: personalAccessToken + - name: AZP_POOL + value: {{.PoolName}} + volumeMounts: + - mountPath: /var/run/docker.sock + name: docker-volume + volumes: + - name: docker-volume + hostPath: + path: /var/run/docker.sock +` + + triggerAuthTemplate = ` +apiVersion: keda.sh/v1alpha1 +kind: TriggerAuthentication +metadata: + name: {{.TriggerAuthName}} + namespace: {{.TestNamespace}} +spec: + secretTargetRef: + - parameter: clientSecret + name: {{.ScalerSecretName}} + key: clientSecret +` + + poolIDScaledObjectTemplate = ` +apiVersion: keda.sh/v1alpha1 +kind: ScaledObject +metadata: + name: {{.ScaledObjectName}} + namespace: {{.TestNamespace}} +spec: + scaleTargetRef: + name: {{.DeploymentName}} + minReplicaCount: {{.MinReplicaCount}} + maxReplicaCount: {{.MaxReplicaCount}} + pollingInterval: 15 + cooldownPeriod: 5 + triggers: + - type: azure-pipelines + metadata: + organizationURLFromEnv: "AZP_URL" + activationTargetPipelinesQueueLength: "1" + poolID: "{{.PoolID}}" + clientId: {{.AzureADClientID}} + tenantId: {{.AzureADTenantID}} + authenticationRef: + name: {{.TriggerAuthName}} +` + + poolNameScaledObjectTemplate = ` +apiVersion: keda.sh/v1alpha1 +kind: ScaledObject +metadata: + name: {{.ScaledObjectName}} + namespace: {{.TestNamespace}} +spec: + scaleTargetRef: + name: {{.DeploymentName}} + minReplicaCount: {{.MinReplicaCount}} + maxReplicaCount: {{.MaxReplicaCount}} + pollingInterval: 15 + cooldownPeriod: 5 + triggers: + - type: azure-pipelines + metadata: + organizationURLFromEnv: "AZP_URL" + activationTargetPipelinesQueueLength: "1" + poolName: "{{.PoolName}}" + clientId: {{.AzureADClientID}} + tenantId: {{.AzureADTenantID}} + authenticationRef: + name: {{.TriggerAuthName}} +` +) + +func TestScaler(t *testing.T) { + t.Log("--- setting up ---") + require.NotEmpty(t, organizationURL, "AZURE_DEVOPS_ORGANIZATION_URL env variable is required for azure pipelines test") + require.NotEmpty(t, personalAccessToken, "AZURE_DEVOPS_PAT env variable is required for azure pipelines test") + require.NotEmpty(t, project, "AZURE_DEVOPS_PROJECT env variable is required for azure pipelines test") + require.NotEmpty(t, buildID, "AZURE_DEVOPS_BUILD_DEFINITION_ID env variable is required for azure pipelines test") + require.NotEmpty(t, poolName, "AZURE_DEVOPS_POOL_NAME env variable is required for azure pipelines test") + require.NotEmpty(t, azureADClientID, "TF_AZURE_SP_APP_ID env variable is required for azure pipelines SPN test") + require.NotEmpty(t, azureADSecret, "AZURE_SP_KEY env variable is required for azure pipelines SPN test") + require.NotEmpty(t, azureADTenantID, "TF_AZURE_SP_TENANT env variable is required for azure pipelines SPN test") + + connection := azuredevops.NewPatConnection(organizationURL, personalAccessToken) + clearAllBuilds(t, connection) + poolID = fmt.Sprintf("%d", getAzDoPoolID(t, connection)) + + kc := GetKubernetesClient(t) + data, templates := getTemplateData() + CreateKubernetesResources(t, kc, testNamespace, data, templates) + + WaitForPodCountInNamespace(t, kc, testNamespace, minReplicaCount, 60, 2) + + testActivation(t, kc, connection) + testScaleOut(t, kc, connection) + testScaleIn(t, kc) + + KubectlApplyWithTemplate(t, data, "poolNameScaledObjectTemplate", poolNameScaledObjectTemplate) + testActivation(t, kc, connection) + testScaleOut(t, kc, connection) + testScaleIn(t, kc) + + DeleteKubernetesResources(t, testNamespace, data, templates) +} + +func getAzDoPoolID(t *testing.T, connection *azuredevops.Connection) int { + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + defer cancel() + taskClient, err := taskagent.NewClient(ctx, connection) + if err != nil { + t.Error(fmt.Sprintf("unable to create task agent client: %s", err.Error()), err) + } + args := taskagent.GetAgentPoolsArgs{ + PoolName: &poolName, + } + pools, err := taskClient.GetAgentPools(ctx, args) + if err != nil { + t.Errorf("unable to get the pools") + } + return *(*pools)[0].Id +} + +func queueBuild(t *testing.T, connection *azuredevops.Connection) { + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + defer cancel() + buildClient, err := build.NewClient(ctx, connection) + if err != nil { + t.Error(fmt.Sprintf("unable to create build client: %s", err.Error()), err) + } + id, err := strconv.Atoi(buildID) + if err != nil { + t.Errorf("unable to parse buildID") + } + args := build.QueueBuildArgs{ + Project: &project, + Build: &build.Build{ + Definition: &build.DefinitionReference{ + Id: &id, + }, + }, + } + _, err = buildClient.QueueBuild(ctx, args) + if err != nil { + t.Errorf("unable to get the pools") + } +} + +func clearAllBuilds(t *testing.T, connection *azuredevops.Connection) { + ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) + defer cancel() + buildClient, err := build.NewClient(ctx, connection) + if err != nil { + t.Error(fmt.Sprintf("unable to create build client: %s", err.Error()), err) + } + var top = 20 + args := build.GetBuildsArgs{ + Project: &project, + StatusFilter: &build.BuildStatusValues.All, + QueryOrder: &build.BuildQueryOrderValues.QueueTimeDescending, + Top: &top, + } + azBuilds, err := buildClient.GetBuilds(ctx, args) + if err != nil { + t.Errorf("unable to get builds") + } + for _, azBuild := range azBuilds.Value { + azBuild.Status = &build.BuildStatusValues.Cancelling + args := build.UpdateBuildArgs{ + Build: &azBuild, + Project: &project, + BuildId: azBuild.Id, + } + _, err = buildClient.UpdateBuild(ctx, args) + if err != nil { + t.Errorf("unable to cancel build") + } + } +} + +func getTemplateData() (templateData, []Template) { + base64Pat := base64.StdEncoding.EncodeToString([]byte(personalAccessToken)) + base64ClientSecret := base64.StdEncoding.EncodeToString([]byte(azureADSecret)) + + return templateData{ + TestNamespace: testNamespace, + AgentSecretName: agentSecretName, + ScalerSecretName: scalerSecretName, + TriggerAuthName: triggerAuthName, + DeploymentName: deploymentName, + ScaledObjectName: scaledObjectName, + MinReplicaCount: fmt.Sprintf("%v", minReplicaCount), + MaxReplicaCount: fmt.Sprintf("%v", maxReplicaCount), + Pat: base64Pat, + URL: organizationURL, + PoolName: poolName, + PoolID: poolID, + AzureADClientID: azureADClientID, + AzureADSecret: base64ClientSecret, + AzureADTenantID: azureADTenantID, + }, []Template{ + {Name: "agentSecretTemplate", Config: agentSecretTemplate}, + {Name: "scalerSecretTemplate", Config: scalerSecretTemplate}, + {Name: "deploymentTemplate", Config: deploymentTemplate}, + {Name: "triggerAuthTemplate", Config: triggerAuthTemplate}, + {Name: "poolIDScaledObjectTemplate", Config: poolIDScaledObjectTemplate}, + } +} + +func testActivation(t *testing.T, kc *kubernetes.Clientset, connection *azuredevops.Connection) { + t.Log("--- testing activation ---") + queueBuild(t, connection) + AssertReplicaCountNotChangeDuringTimePeriod(t, kc, deploymentName, testNamespace, minReplicaCount, 60) +} + +func testScaleOut(t *testing.T, kc *kubernetes.Clientset, connection *azuredevops.Connection) { + t.Log("--- testing scale out ---") + queueBuild(t, connection) + assert.True(t, WaitForDeploymentReplicaReadyCount(t, kc, deploymentName, testNamespace, maxReplicaCount, 60, 1), + "replica count should be 2 after 1 minute") +} + +func testScaleIn(t *testing.T, kc *kubernetes.Clientset) { + t.Log("--- testing scale in ---") + assert.True(t, WaitForPodCountInNamespace(t, kc, testNamespace, minReplicaCount, 60, 5), + "pod count should be 0 after 1 minute") +}