feat: add GCP Parameter Manager OpenFeature provider#1771
feat: add GCP Parameter Manager OpenFeature provider#1771mahpatil wants to merge 1 commit intoopen-feature:mainfrom
Conversation
Adds a new OpenFeature provider for GCP Parameter Manager that enables reading feature flags from GCP Parameter Manager secrets. Includes the provider implementation with flag caching, value conversion, unit tests, integration tests, and a sample application demonstrating usage.
There was a problem hiding this comment.
Code Review
This pull request introduces a new OpenFeature provider for Google Cloud Parameter Manager, allowing feature flags to be evaluated from GCP parameters. The provider supports various data types (boolean, string, integer, double, and JSON objects) and includes an in-memory cache for performance. The changes encompass the core provider implementation, configuration options, unit and integration tests, and a sample application demonstrating its usage. Feedback indicates a potential race condition in the FlagCache.get method, suggesting a more robust removal mechanism for expired entries, and points out that the spotbugs-exclusions.xml file contains irrelevant entries that should be removed for clarity.
| if (entry.isExpired()) { | ||
| store.remove(key); | ||
| return Optional.empty(); | ||
| } |
There was a problem hiding this comment.
There is a race condition in the get method. If a thread identifies an entry as expired and calls store.remove(key), it might inadvertently remove a fresh value inserted by another thread between the isExpired() check and the remove() call. Using store.remove(key, entry) ensures that the entry is only removed if it matches the one that was found to be expired.
| if (entry.isExpired()) { | |
| store.remove(key); | |
| return Optional.empty(); | |
| } | |
| if (entry.isExpired()) { | |
| store.remove(key, entry); | |
| return Optional.empty(); | |
| } |
| <!-- Ignore generated dev.openfeature.flagd.grpc schemas --> | ||
| <Match> | ||
| <Package name="~dev\.openfeature\.flagd\.grpc.*" /> | ||
| </Match> | ||
|
|
||
| <!-- Ignore generated dev.openfeature.flagd.sync schemas --> | ||
| <Match> | ||
| <Package name="~dev\.openfeature\.flagd\.sync.*" /> | ||
| </Match> | ||
|
|
||
| <!-- Allow predictable random for backoff jitter generation --> | ||
| <Match> | ||
| <Class name="dev.openfeature.contrib.providers.flagd.resolver.common.backoff.BackoffService" /> | ||
| <Bug pattern="PREDICTABLE_RANDOM" /> | ||
| </Match> | ||
|
|
||
| <!-- Suppress: EI_EXPOSE_REP2 in ModuleMachine --> | ||
| <Match> | ||
| <Class name="dev.openfeature.contrib.providers.gofeatureflag.wasm.ModuleMachine"/> | ||
| <Bug pattern="EI_EXPOSE_REP2"/> | ||
| </Match> | ||
|
|
||
| <!-- Suppress: SKIPPED_CLASS_TOO_BIG in ModuleMachineFuncGroup_0 --> | ||
| <Match> | ||
| <Class name="dev.openfeature.contrib.providers.gofeatureflag.wasm.ModuleMachineFuncGroup_0"/> | ||
| <Bug pattern="SKIPPED_CLASS_TOO_BIG"/> | ||
| </Match> |
There was a problem hiding this comment.
This spotbugs-exclusions.xml file contains several exclusions for packages and classes (e.g., dev.openfeature.flagd.grpc, BackoffService, ModuleMachine) that are not part of this module. It appears to have been copied from another provider without being cleaned up. Please remove these irrelevant entries to maintain clarity.
Summary
samples/gcp-parameter-manager-sample/with setup/teardown scriptsProvider Details
providers/gcp-parameter-managerGcpParameterManagerProviderTest plan
FlagCache,FlagValueConverter, andGcpParameterManagerProviderGcpParameterManagerProviderIntegrationTest) requires a real GCP project — setGCP_PROJECT_IDenv var to runsamples/gcp-parameter-manager-sample/README.mdto run end-to-end