Skip to content
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

Provide validation for identityId with Azure pod identity & Azure AD Workload Identity in TriggerAuthentication #4696

Merged
merged 41 commits into from
Aug 29, 2023
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
41 commits
Select commit Hold shift + click to select a range
9c9af60
Update
SpiritZhou Jun 16, 2023
4857d65
Update CHANGELOG.md
SpiritZhou Jun 19, 2023
15f0d6e
Merge branch 'main' into spiritzhou/checkemptyidentityid
SpiritZhou Jun 30, 2023
bcea677
Merge branch 'main' into spiritzhou/checkemptyidentityid
SpiritZhou Jul 3, 2023
6edb425
Update
SpiritZhou Jul 3, 2023
70c5a14
Add webhooks
SpiritZhou Jul 5, 2023
c74f164
Update
SpiritZhou Jul 6, 2023
31178f2
Update
SpiritZhou Jul 6, 2023
ab3d9d0
Merge branch 'main' into spiritzhou/checkemptyidentityid
SpiritZhou Jul 6, 2023
966fb35
Update
SpiritZhou Jul 6, 2023
e43e3f0
fix
SpiritZhou Jul 6, 2023
1898b7b
Update
SpiritZhou Jul 6, 2023
294a0b8
Update
SpiritZhou Jul 6, 2023
8f13085
Update
SpiritZhou Jul 6, 2023
5de4f46
Add e2e test
SpiritZhou Jul 7, 2023
42f14a1
Fix
SpiritZhou Jul 7, 2023
9538860
Update
SpiritZhou Jul 10, 2023
9847f4b
Merge branch 'main' into spiritzhou/checkemptyidentityid
SpiritZhou Jul 10, 2023
0516b70
Update CHANGELOG.md
SpiritZhou Jul 10, 2023
113b115
Update
SpiritZhou Jul 10, 2023
5b95650
Update
SpiritZhou Jul 12, 2023
a183528
Update pkg/scaling/resolver/scale_resolvers.go
SpiritZhou Jul 13, 2023
8126a33
Update
SpiritZhou Jul 13, 2023
dbebad4
Merge branch 'main' into spiritzhou/checkemptyidentityid
SpiritZhou Jul 17, 2023
e00f693
Update
SpiritZhou Jul 19, 2023
ccc4bdc
Update
SpiritZhou Jul 19, 2023
b7ce1d9
Update
SpiritZhou Aug 18, 2023
2399cb5
Merge branch 'main' into spiritzhou/checkemptyidentityid
SpiritZhou Aug 18, 2023
58d32c8
Update
SpiritZhou Aug 18, 2023
82ee9aa
Update
SpiritZhou Aug 18, 2023
f0bd980
Update
SpiritZhou Aug 18, 2023
12f1fb4
Fix order
SpiritZhou Aug 18, 2023
00a359f
Update CHANGELOG.md
SpiritZhou Aug 21, 2023
1d2344c
Update CHANGELOG.md
SpiritZhou Aug 21, 2023
dfa0e7f
Update CHANGELOG.md
SpiritZhou Aug 21, 2023
c00289d
Update CHANGELOG.md
SpiritZhou Aug 21, 2023
0f93a62
Update CHANGELOG.md
SpiritZhou Aug 21, 2023
958d41a
Merge branch 'main' into spiritzhou/checkemptyidentityid
SpiritZhou Aug 25, 2023
78307d7
Update
SpiritZhou Aug 25, 2023
7f51c4b
Merge branch 'main' into spiritzhou/checkemptyidentityid
SpiritZhou Aug 28, 2023
485fe87
Merge branch 'main' into spiritzhou/checkemptyidentityid
tomkerkhove Aug 29, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ To learn more about active deprecations, we recommend checking [GitHub Discussio

### Improvements

- TODO ([#XXX](https://github.com/kedacore/keda/issue/XXX))
- **Azure Pod Identity**: Add validation of identity ID to prevent usage of empty identity ID ([#4528](https://github.com/kedacore/keda/issues/4528))

### Fixes

Expand Down
10 changes: 9 additions & 1 deletion apis/keda/v1alpha1/triggerauthentication_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,15 @@ const (
type AuthPodIdentity struct {
Provider PodIdentityProvider `json:"provider"`
// +optional
IdentityID string `json:"identityId"`
IdentityID *string `json:"identityId"`
}

func (a *AuthPodIdentity) GetIdentityID() string {
if a.IdentityID == nil {
return ""
} else {
return *a.IdentityID
}
}

// AuthSecretTargetRef is used to authenticate using a reference to a secret
Expand Down
4 changes: 2 additions & 2 deletions pkg/scalers/azure/azure_app_insights.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,10 +69,10 @@ func getAuthConfig(ctx context.Context, info AppInsightsInfo, podIdentity kedav1
case kedav1alpha1.PodIdentityProviderAzure:
config := auth.NewMSIConfig()
config.Resource = info.AppInsightsResourceURL
config.ClientID = podIdentity.IdentityID
config.ClientID = podIdentity.GetIdentityID()
return config
case kedav1alpha1.PodIdentityProviderAzureWorkload:
return NewAzureADWorkloadIdentityConfig(ctx, podIdentity.IdentityID, info.AppInsightsResourceURL)
return NewAzureADWorkloadIdentityConfig(ctx, podIdentity.GetIdentityID(), info.AppInsightsResourceURL)
}
return nil
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/scalers/azure/azure_data_explorer.go
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ func getDataExplorerAuthConfig(metadata *DataExplorerMetadata) (*kusto.Connectio

case kedav1alpha1.PodIdentityProviderAzure, kedav1alpha1.PodIdentityProviderAzureWorkload:
azureDataExplorerLogger.V(1).Info(fmt.Sprintf("Creating Azure Data Explorer Client using podIdentity %s", metadata.PodIdentity.Provider))
creds, chainedErr := NewChainedCredential(metadata.PodIdentity.IdentityID, metadata.PodIdentity.Provider)
creds, chainedErr := NewChainedCredential(metadata.PodIdentity.GetIdentityID(), metadata.PodIdentity.Provider)
if chainedErr != nil {
return nil, chainedErr
}
Expand Down
4 changes: 2 additions & 2 deletions pkg/scalers/azure/azure_eventhub.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ func GetEventHubClient(ctx context.Context, info EventHubInfo) (*eventhub.Hub, e
envJWTProviderOption := aad.JWTProviderWithAzureEnvironment(&env)
resourceURLJWTProviderOption := aad.JWTProviderWithResourceURI(info.EventHubResourceURL)
clientIDJWTProviderOption := func(config *aad.TokenProviderConfiguration) error {
config.ClientID = info.PodIdentity.IdentityID
config.ClientID = info.PodIdentity.GetIdentityID()
return nil
}

Expand All @@ -68,7 +68,7 @@ func GetEventHubClient(ctx context.Context, info EventHubInfo) (*eventhub.Hub, e
// User wants to use AAD Workload Identity
env := azure.Environment{ActiveDirectoryEndpoint: info.ActiveDirectoryEndpoint, ServiceBusEndpointSuffix: info.ServiceBusEndpointSuffix}
hubEnvOptions := eventhub.HubWithEnvironment(env)
provider := NewAzureADWorkloadIdentityTokenProvider(ctx, info.PodIdentity.IdentityID, info.EventHubResourceURL)
provider := NewAzureADWorkloadIdentityTokenProvider(ctx, info.PodIdentity.GetIdentityID(), info.EventHubResourceURL)

return eventhub.NewHub(info.Namespace, info.EventHubName, provider, hubEnvOptions)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ func TryAndGetAzureManagedPrometheusHTTPRoundTripper(podIdentity kedav1alpha1.Au
return nil, fmt.Errorf("trigger metadata cannot be nil")
}

chainedCred, err := NewChainedCredential(podIdentity.IdentityID, podIdentity.Provider)
chainedCred, err := NewChainedCredential(podIdentity.GetIdentityID(), podIdentity.Provider)
if err != nil {
return nil, err
}
Expand Down
4 changes: 2 additions & 2 deletions pkg/scalers/azure/azure_monitor.go
Original file line number Diff line number Diff line change
Expand Up @@ -89,11 +89,11 @@ func createMetricsClient(ctx context.Context, info MonitorInfo, podIdentity keda
case kedav1alpha1.PodIdentityProviderAzure:
config := auth.NewMSIConfig()
config.Resource = info.AzureResourceManagerEndpoint
config.ClientID = podIdentity.IdentityID
config.ClientID = *podIdentity.IdentityID
SpiritZhou marked this conversation as resolved.
Show resolved Hide resolved

authConfig = config
case kedav1alpha1.PodIdentityProviderAzureWorkload:
authConfig = NewAzureADWorkloadIdentityConfig(ctx, podIdentity.IdentityID, info.AzureResourceManagerEndpoint)
authConfig = NewAzureADWorkloadIdentityConfig(ctx, *podIdentity.IdentityID, info.AzureResourceManagerEndpoint)
}

authorizer, _ := authConfig.Authorizer()
Expand Down
8 changes: 4 additions & 4 deletions pkg/scalers/azure/azure_storage.go
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ func ParseAzureStorageQueueConnection(ctx context.Context, httpClient util.HTTPD

return credential, endpoint, nil
default:
return nil, nil, fmt.Errorf("azure queues doesn't support %s pod identity type", podIdentity)
return nil, nil, fmt.Errorf("azure queues doesn't support %s pod identity type", podIdentity.Provider)
}
}

Expand Down Expand Up @@ -139,7 +139,7 @@ func ParseAzureStorageBlobConnection(ctx context.Context, httpClient util.HTTPDo

return credential, endpoint, nil
default:
return nil, nil, fmt.Errorf("azure queues doesn't support %s pod identity type", podIdentity)
return nil, nil, fmt.Errorf("azure queues doesn't support %s pod identity type", podIdentity.Provider)
}
}

Expand Down Expand Up @@ -207,9 +207,9 @@ func parseAccessTokenAndEndpoint(ctx context.Context, httpClient util.HTTPDoer,

switch podIdentity.Provider {
case kedav1alpha1.PodIdentityProviderAzure:
token, err = GetAzureADPodIdentityToken(ctx, httpClient, podIdentity.IdentityID, storageResource)
token, err = GetAzureADPodIdentityToken(ctx, httpClient, podIdentity.GetIdentityID(), storageResource)
case kedav1alpha1.PodIdentityProviderAzureWorkload:
token, err = GetAzureADWorkloadIdentityToken(ctx, podIdentity.IdentityID, storageResource)
token, err = GetAzureADWorkloadIdentityToken(ctx, podIdentity.GetIdentityID(), storageResource)
}

if err != nil {
Expand Down
2 changes: 1 addition & 1 deletion pkg/scalers/azure_blob_scaler.go
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,7 @@ func parseAzureBlobMetadata(config *ScalerConfig, logger logr.Logger) (*azure.Bl
return nil, kedav1alpha1.AuthPodIdentity{}, fmt.Errorf("no accountName given")
}
default:
return nil, kedav1alpha1.AuthPodIdentity{}, fmt.Errorf("pod identity %s not supported for azure storage blobs", config.PodIdentity)
return nil, kedav1alpha1.AuthPodIdentity{}, fmt.Errorf("pod identity %s not supported for azure storage blobs", config.PodIdentity.Provider)
}

meta.ScalerIndex = config.ScalerIndex
Expand Down
8 changes: 4 additions & 4 deletions pkg/scalers/azure_log_analytics_scaler.go
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,7 @@ func parseAzureLogAnalyticsMetadata(config *ScalerConfig) (*azureLogAnalyticsMet
case kedav1alpha1.PodIdentityProviderAzure, kedav1alpha1.PodIdentityProviderAzureWorkload:
meta.podIdentity = config.PodIdentity
default:
return nil, fmt.Errorf("error parsing metadata. Details: Log Analytics Scaler doesn't support pod identity %s", config.PodIdentity)
return nil, fmt.Errorf("error parsing metadata. Details: Log Analytics Scaler doesn't support pod identity %s", config.PodIdentity.Provider)
}

// Getting workspaceId
Expand Down Expand Up @@ -480,7 +480,7 @@ func (s *azureLogAnalyticsScaler) getAuthorizationToken(ctx context.Context) (to

switch s.metadata.podIdentity.Provider {
case kedav1alpha1.PodIdentityProviderAzureWorkload:
aadToken, err := azure.GetAzureADWorkloadIdentityToken(ctx, s.metadata.podIdentity.IdentityID, s.metadata.logAnalyticsResourceURL)
aadToken, err := azure.GetAzureADWorkloadIdentityToken(ctx, s.metadata.podIdentity.GetIdentityID(), s.metadata.logAnalyticsResourceURL)
if err != nil {
return tokenData{}, nil
}
Expand Down Expand Up @@ -565,10 +565,10 @@ func (s *azureLogAnalyticsScaler) executeAADApicall(ctx context.Context) ([]byte

func (s *azureLogAnalyticsScaler) executeIMDSApicall(ctx context.Context) ([]byte, int, error) {
var urlStr string
if s.metadata.podIdentity.IdentityID == "" {
if s.metadata.podIdentity.GetIdentityID() == "" {
urlStr = fmt.Sprintf(azure.MSIURL, s.metadata.logAnalyticsResourceURL)
} else {
urlStr = fmt.Sprintf(azure.MSIURLWithClientID, s.metadata.logAnalyticsResourceURL, url.QueryEscape(s.metadata.podIdentity.IdentityID))
urlStr = fmt.Sprintf(azure.MSIURLWithClientID, s.metadata.logAnalyticsResourceURL, url.QueryEscape(s.metadata.podIdentity.GetIdentityID()))
}

request, err := http.NewRequestWithContext(ctx, http.MethodGet, urlStr, nil)
Expand Down
2 changes: 1 addition & 1 deletion pkg/scalers/azure_monitor_scaler.go
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,7 @@ func parseAzurePodIdentityParams(config *ScalerConfig) (clientID string, clientP
case kedav1alpha1.PodIdentityProviderAzure, kedav1alpha1.PodIdentityProviderAzureWorkload:
// no params required to be parsed
default:
return "", "", fmt.Errorf("azure Monitor doesn't support pod identity %s", config.PodIdentity)
return "", "", fmt.Errorf("azure Monitor doesn't support pod identity %s", config.PodIdentity.Provider)
}

return clientID, clientPassword, nil
Expand Down
2 changes: 1 addition & 1 deletion pkg/scalers/azure_queue_scaler.go
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,7 @@ func parseAzureQueueMetadata(config *ScalerConfig, logger logr.Logger) (*azureQu
return nil, kedav1alpha1.AuthPodIdentity{}, fmt.Errorf("no accountName given")
}
default:
return nil, kedav1alpha1.AuthPodIdentity{}, fmt.Errorf("pod identity %s not supported for azure storage queues", config.PodIdentity)
return nil, kedav1alpha1.AuthPodIdentity{}, fmt.Errorf("pod identity %s not supported for azure storage queues", config.PodIdentity.Provider)
}

meta.scalerIndex = config.ScalerIndex
Expand Down
4 changes: 2 additions & 2 deletions pkg/scalers/azure_servicebus_scaler.go
Original file line number Diff line number Diff line change
Expand Up @@ -214,7 +214,7 @@ func parseAzureServiceBusMetadata(config *ScalerConfig, logger logr.Logger) (*az
}

default:
return nil, fmt.Errorf("azure service bus doesn't support pod identity %s", config.PodIdentity)
return nil, fmt.Errorf("azure service bus doesn't support pod identity %s", config.PodIdentity.Provider)
}

meta.scalerIndex = config.ScalerIndex
Expand Down Expand Up @@ -297,7 +297,7 @@ func (s *azureServiceBusScaler) getServiceBusAdminClient() (*admin.Client, error
case "", kedav1alpha1.PodIdentityProviderNone:
client, err = admin.NewClientFromConnectionString(s.metadata.connection, nil)
case kedav1alpha1.PodIdentityProviderAzure, kedav1alpha1.PodIdentityProviderAzureWorkload:
creds, chainedErr := azure.NewChainedCredential(s.podIdentity.IdentityID, s.podIdentity.Provider)
creds, chainedErr := azure.NewChainedCredential(s.podIdentity.GetIdentityID(), s.podIdentity.Provider)
if chainedErr != nil {
return nil, chainedErr
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/scalers/rabbitmq_scaler.go
Original file line number Diff line number Diff line change
Expand Up @@ -242,7 +242,7 @@ func parseRabbitMQMetadata(config *ScalerConfig) (*rabbitMQMetadata, error) {

if config.PodIdentity.Provider == v1alpha1.PodIdentityProviderAzureWorkload {
if config.AuthParams["workloadIdentityResource"] != "" {
meta.workloadIdentityClientID = config.PodIdentity.IdentityID
meta.workloadIdentityClientID = *config.PodIdentity.IdentityID
SpiritZhou marked this conversation as resolved.
Show resolved Hide resolved
meta.workloadIdentityResource = config.AuthParams["workloadIdentityResource"]
}
}
Expand Down
5 changes: 3 additions & 2 deletions pkg/scalers/rabbitmq_scaler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (
"github.com/stretchr/testify/assert"

"github.com/kedacore/keda/v2/apis/keda/v1alpha1"
kedautil "github.com/kedacore/keda/v2/pkg/util"
)

const (
Expand Down Expand Up @@ -152,9 +153,9 @@ var testRabbitMQAuthParamData = []parseRabbitMQAuthParamTestData{
// failure, TLS invalid
{map[string]string{"queueName": "sample", "hostFromEnv": host}, v1alpha1.AuthPodIdentity{}, map[string]string{"tls": "yes", "ca": "caaa", "cert": "ceert", "key": "kee"}, true, true, false},
// success, WorkloadIdentity
{map[string]string{"queueName": "sample", "hostFromEnv": host, "protocol": "http"}, v1alpha1.AuthPodIdentity{Provider: v1alpha1.PodIdentityProviderAzureWorkload, IdentityID: "client-id"}, map[string]string{"workloadIdentityResource": "rabbitmq-resource-id"}, false, false, true},
{map[string]string{"queueName": "sample", "hostFromEnv": host, "protocol": "http"}, v1alpha1.AuthPodIdentity{Provider: v1alpha1.PodIdentityProviderAzureWorkload, IdentityID: kedautil.StringPointer("client-id")}, map[string]string{"workloadIdentityResource": "rabbitmq-resource-id"}, false, false, true},
// failure, WoekloadIdentity not supported for amqp
{map[string]string{"queueName": "sample", "hostFromEnv": host, "protocol": "amqp"}, v1alpha1.AuthPodIdentity{Provider: v1alpha1.PodIdentityProviderAzureWorkload, IdentityID: "client-id"}, map[string]string{"workloadIdentityResource": "rabbitmq-resource-id"}, true, false, false},
{map[string]string{"queueName": "sample", "hostFromEnv": host, "protocol": "amqp"}, v1alpha1.AuthPodIdentity{Provider: v1alpha1.PodIdentityProviderAzureWorkload, IdentityID: kedautil.StringPointer("client-id")}, map[string]string{"workloadIdentityResource": "rabbitmq-resource-id"}, true, false, false},
}
var rabbitMQMetricIdentifiers = []rabbitMQMetricIdentifier{
{&testRabbitMQMetadata[1], 0, "s0-rabbitmq-sample"},
Expand Down
6 changes: 3 additions & 3 deletions pkg/scaling/resolver/azure_keyvault_handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -133,12 +133,12 @@ func (vh *AzureKeyVaultHandler) getAuthConfig(ctx context.Context, client client
case kedav1alpha1.PodIdentityProviderAzure:
config := auth.NewMSIConfig()
config.Resource = keyVaultResourceURL
config.ClientID = podIdentity.IdentityID
config.ClientID = podIdentity.GetIdentityID()

return config, nil
case kedav1alpha1.PodIdentityProviderAzureWorkload:
return azure.NewAzureADWorkloadIdentityConfig(ctx, podIdentity.IdentityID, keyVaultResourceURL), nil
return azure.NewAzureADWorkloadIdentityConfig(ctx, podIdentity.GetIdentityID(), keyVaultResourceURL), nil
default:
return nil, fmt.Errorf("key vault does not support pod identity provider - %s", podIdentity)
return nil, fmt.Errorf("key vault does not support pod identity provider - %s", podIdentity.Provider)
}
}
4 changes: 4 additions & 0 deletions pkg/scaling/resolver/scale_resolvers.go
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,10 @@ func ResolveAuthRefAndPodIdentity(ctx context.Context, client client.Client, log
authParams["awsRoleArn"] = serviceAccount.Annotations[kedav1alpha1.PodIdentityAnnotationEKS]
} else if podIdentity.Provider == kedav1alpha1.PodIdentityProviderAwsKiam {
authParams["awsRoleArn"] = podTemplateSpec.ObjectMeta.Annotations[kedav1alpha1.PodIdentityAnnotationKiam]
} else if podIdentity.Provider == kedav1alpha1.PodIdentityProviderAzure {
if podIdentity.GetIdentityID() == "" {
logger.Info("WARNING: IdentityID of PodIdentity is empty or nil")
}
SpiritZhou marked this conversation as resolved.
Show resolved Hide resolved
}
return authParams, podIdentity, nil
}
Expand Down
22 changes: 22 additions & 0 deletions pkg/util/conver_types.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
/*
Copyright 2023 The KEDA 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 util

// String returns a pointer to the string value passed in.
func StringPointer(v string) *string {
return &v
}