Keda: Renaming metadata propertied to indicate source of the value

[ahmelsayed]

Today in the metadata section of each scaler, there are some properties that are intended to be resolved from the scaleTarget of the ScaledObject. These are mostly meant to refer to the secrets needed (if any) for each scaler to access the event source.

For example aws-sqs-queue looks something like this

...
    type: aws-sqs-queue
    metadata:
      awsAccessKeyID: AWS_ACCESS_KEY_ID
      awsSecretAccessKey: AWS_SECRET_ACCESS_KEY

This means that the scaleTarget (e.g: Deployment) will have a container that has those defined as environment variables (whether directly or sourced from a v1.Secret or a v1.ConfigMap). KEDA resolves those values and passes the actual value of the key/secret in this example to the aws-sqs-queue scaler.

Based on feedback and some discussion on the community meeting, we are considering renaming all metadata keys that behave in that way with a standard suffix, for example *FromEnv

This is a look at all such settings for all scalers.

| Scaler | values | update |
|--------|--------|--------|
| azure-blob | connection (Default: AzureWebJobsStorage) | connectionFromEnv |
| azure-monitor | activeDirectoryClientId
activeDirectoryClientPassword | activeDirectoryClientIdFromEnv
activeDirectoryClientPasswordFromEnv |
| azure-queue | connection (Default: AzureWebJobsStorage) | connectionFromEnv |
| azure-servicebus | connection | connectionFromEnv |
| azure-eventhub | storageConnection (Default: AzureWebJobsStorage)
connection (Default: EventHub) | storageConnectionFromEnv
connectionFromEnv |
| aws-sqs-queue
aws-cloudwatch
aws-kinesis-stream | awsAccessKeyID (Default: AWS_ACCESS_KEY_ID)
awsSecretAccessKey (Default: AWS_SECRET_ACCESS_KEY) | awsAccessKeyIDFromEnv
awsSecretAccessKeyFromEnv |
| kafka | _(none)_ | _(none)_
| rabbitmq | apiHost
host | apiHostFromEnv
host |
| prometheus | _(none)_ | _(none)_ |
| cron | _(none)_ | _(none)_ |
| redis
redis-streams | address
host
port
password | addressFromEnv
hostFromEnv
port
passwordFromEnv
| gcp-pubsub | credentials | credentialsFromEnv |
| external | _(any matching value)_ | _(any matching value with FromEnv suffix)_
| liiklus | _(none)_ | _(none)_ |
| stan | _(none)_ | _(none)_ |
| huawei-cloudeye | | _(none)_ | _(none)_ |
| postgresql | connection
password | connectionFromEnv
passwordFromEnv |
| mysql | connectionString
password | connectionStringFromEnv
passwordFromEnv |

There is also probably a chance to make things a bit more consistent between the scalers as a popular ask from @tomkerkhove :)

[tomkerkhove]

This relates to https://github.com/kedacore/keda/issues/753#issuecomment-656224125, right?

[tomkerkhove]

This looks ok to me but only wonder why we are removing the port for Redis because we still have address and host. A host without a port seems a bit meaningless, but maybe I'm missing something?

Another thing I'm wondering is, now that we have the prefixes that's great! But will we keep the old values around for literal config values? If it's up to me I'd say yes, definitely for things such as host, address, client id, etc.

What do you think @ahmelsayed ?

Adding @mcollier who opened https://github.com/kedacore/keda/issues/753, would this be sufficient for you or not?

[zroubalik]

Another thing I'm wondering is, now that we have the prefixes that's great! But will we keep the old values around for literal config values? If it's up to me I'd say yes, definitely for things such as host, address, client id, etc.

+1

[ahmelsayed]

regarding port, it just felt odd that we were resolving the port as a secret as it's just a port number so I removed it. I'm fine if we have one that can be resolved from the environment as well, but it just seemed unnecessary

[tomkerkhove]

I fully agree with that! Certainly since we now use the FromEnv suffix.

Now that we are changing that, will we support all of those through TriggerAuthentication as well? I think these would be super convenient but up to you folks.

[zroubalik]

@ahmelsayed do you have an estimate on implementing this change, please? So we can coordinate the follow up steps re Beta release. Thanks!

[ahmelsayed]

I'm finishing it up today. Sorry for the delay

[turbaszek]

Should we update related scalers' examples?

[tomkerkhove]

No worries and thanks for the update @ahmelsayed !

@turbaszek yes, they will all be updated.

[zroubalik]

@ahmelsayed no worries and no rush, just checking the status.Thanks for the update

[tomkerkhove]

Re-opening as docs are not done yet

[tomkerkhove]

@ahmelsayed Might be good to update https://github.com/kedacore/keda-docs/blob/master/content/docs/2.0/migration.md as well, either through https://github.com/kedacore/keda-docs/pull/241 or a separate one.