Skip to content

Conversation

m-Bilal
Copy link
Member

@m-Bilal m-Bilal commented Mar 19, 2025

About

Completes ENT-290

This PR adds support for a credentials provider as per this RFC

Changes

Required New Env Vars Introduced

  • ELASTICSEARCH_CREDENTIALS_PROVIDER_KEY: This is the key for the credentials provider
  • ELASTICSEARCH_CREDENTIALS_PROVIDER_MECHANISM: This is the security credential that is expected from the credential provider service. Could be api-key or service-token
  • HASURA_CREDENTIALS_PROVIDER_URI: The URI for the webhook service
    All these env vars are not part of the ddn connector init flow. The user needs to manually add them after initializing the connector.

When to use Credentials Provider

The Connector will use the Credentials Provider service if HASURA_CREDENTIALS_PROVIDER_URI env var is set and is not empty

Tests

Basic unit tests that test the assertion of correct errors are present, but a full test would require a running elasticsearch instance and an environment where variables can be set and unset. These things are not a part of the test suite for now, but will be in the future.

errCredentialProviderKeyNotSet = fmt.Errorf("%s is not set", credentailsProviderKeyEnvVar)
errCredentialProviderMechanismNotSet = fmt.Errorf("%s is not set", credentailsProviderMechanismEnvVar)
errCredentialProviderMechanismInvalid = fmt.Errorf("invalid value for %s, should be either \"api-key\" or \"service-token\"", credentailsProviderMechanismEnvVar)
)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not a go programmer are these required env vars now or only if you want to use this new get creds function?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am assuming it is only required if HASURA_CREDENTIALS_PROVIDER_URI is set.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@codedmart your understanding is correct. These are in general not required, but if HASURA_CREDENTIALS_PROVIDER_URI is set, then these are required

@m-Bilal m-Bilal merged commit 821135d into main Mar 26, 2025
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants