Contributor Guidelines

[joclegg]

Hi,

We are looking to possibly use this in our platform however we are currently migrating away from using connection strings with cosmos db and instead using Azure AD. We can add the option to use this constructor for the cosmos clients that get created and then just give the scaler service the correct role assignments for access.

I would like to contribute this and submit a PR. Are there any guidelines for contributing?

[JatinSanghvi]

@joclegg, we have the guidelines mentioned in https://github.com/kedacore/external-scaler-azure-cosmos-db/blob/main/CONTRIBUTING.md. How would you be generating the token credentials to be passed to the CosmosClient constructor? Would it be username/password or Client ID/secret. I believe that will make a difference in what metadata properties the updated trigger specification would be containing.

EDIT: I think you will need to add three properties: tenantId, clientId and clientSecret as in https://keda.sh/docs/scalers/azure-log-analytics/#trigger-specification.

[JatinSanghvi]

@tomkerkhove, do you have any comments on adding AAD support? For instance, AFAIK, the TriggerAuthentication cannot be referenced inside external scaler's trigger specification, so client ID and secret will be present in the ScaledObject in plain text, which would be a bigger issue than connection string since connection string only provide access to the target resource, not to all the resources the user or service principal has access to. There might be more things to be concerned about other than security.

[tomkerkhove]

I think it's a valuable request and we should:

1. Open an issue to evaluate exposing TA to external scalers
2. We can rely on environment variables for now AFAIK, no? For example clientSecretFromEnv and the rest can be regular fields given those are not secrets.

[joclegg]

Unfortunately I hadn't really thought about the different token mechanisms for Azure. I was just hoping to get support for AAD pod identity and longer term (when it is released from preview) for workload identities. So that we don't need to have secrets anywhere in source. I guess it looks like the other Azure scalers tend to support client id, client secret and tenant id so you might want to support that too. But if you are uncomfortable with the latter then I think support for the manage identity auth should not be an issue from a security perspective. If anything that would be the best authentication option.

[tomkerkhove]

We tend to give end-users option so I would strive to support all scenarios similar to the other scalers indeed

[joclegg]

I guess would good next steps just be to raise an issue against this repo about supporting AD connections?

[tomkerkhove]

I'd create one per scenario so things can be prioritized separately but that's a good idea indeed