-
Notifications
You must be signed in to change notification settings - Fork 2
Support feature flags in the CI #209
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
base: main
Are you sure you want to change the base?
Conversation
536a5fc to
d7a0c79
Compare
4355fe1 to
3dea7c7
Compare
f0dcb5e to
be7a4ee
Compare
a7a0f61 to
fa3cd53
Compare
c875cc7 to
7efa4d7
Compare
7efa4d7 to
2d11958
Compare
34ae8dd to
401a746
Compare
chouetz
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
| from dda.feature_flags.manager import FeatureFlagManager | ||
|
|
||
| return FeatureFlagManager(self) | ||
| self.display_info(f"IS IT RUNNING IN CI {running_in_ci()}") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| self.display_info(f"IS IT RUNNING IN CI {running_in_ci()}") | |
| self.display_info(f"Is it running in CI {running_in_ci()}") |
🥜 the capital letters seems aggressive
| @@ -0,0 +1,6 @@ | |||
| def fetch_secret_ssm(name: str) -> str: | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| def fetch_secret_ssm(name: str) -> str: | |
| def fetch_secret(name: str) -> str: |
As it's in the ssm namespace I guess it's ok. Just to align on the naming from the vault module
| DDA_FEATURE_FLAGS_CI_VAULT_KEY: CLIENT_TOKEN | ||
| DDA_FEATURE_FLAGS_CI_VAULT_PATH_MACOS: kv/path/to/dda/feature-flags | ||
| DDA_FEATURE_FLAGS_CI_VAULT_KEY_MACOS: CLIENT_TOKEN | ||
| DDA_FEATURE_FLAGS_CI_SSM_KEY_WINDOWS: my/ssm/parameter/name |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| DDA_FEATURE_FLAGS_CI_SSM_KEY_WINDOWS: my/ssm/parameter/name | |
| DDA_FEATURE_FLAGS_CI_SSM_KEY_WINDOWS: ci.repo-name.my-ssm-parameter-name |
🥜 but I think the naming is important for the ssm param to be retrieved correctly
Create
LocalFeatureFlagManagervsCIFeatureFlagManager, these are two different subclasses because they are quite different in the way they retrieve secrets, and the context they provide.Also add some documentation about how to use feature flags in actual
ddacode.Using the feature flag in the CI expect you to pass some keys as environment variable. As the way the secrets can be retrieved is not the same on the different OSes and that the parameter name can be different, you need the following environment variables in
.gitlab-ci.yamlto configure token retrieval for dda features flagsDDA_FEATURE_FLAGS_CI_VAULT_KEYDDA_FEATURE_FLAGS_CI_VAULT_PATHDDA_FEATURE_FLAGS_CI_VAULT_KEY_MACOSDDA_FEATURE_FLAGS_CI_VAULT_PATH_MACOSDDA_FEATURE_FLAGS_CI_SSM_KEY_WINDOWSExample of jobs running with that on datadog-agent side: https://gitlab.ddbuild.io/DataDog/datadog-agent/-/jobs/1184492696