Skip to content

feat(gcp): add parameter manager support#1808

Open
mahpatil wants to merge 19 commits into
open-feature:mainfrom
mahpatil:feat/gcp-parameter-manager
Open

feat(gcp): add parameter manager support#1808
mahpatil wants to merge 19 commits into
open-feature:mainfrom
mahpatil:feat/gcp-parameter-manager

Conversation

@mahpatil

@mahpatil mahpatil commented Jun 5, 2026

Copy link
Copy Markdown
Contributor

This PR

Adds a GCP Parameter Manager support to the GCP Provider that reads feature flags from GCP Parameter Manager config.
Includes updates to sample application under samples/gcp/ with setup/teardown script
Package: providers/gcp
Class: GcpParameterManagerProvider
Supports all OpenFeature flag types via structured or plain-text secret values
Configurable poll interval and GCP project settings

Related Issues

Fixes GCP services covered under #1420

Notes

This builds on top of the previous PR that added support for GCP Secret manager.

Follow-up Tasks

How to test

  • Unit tests for GcpParameterManagerProvider
  • Integration test (GcpParameterManagerProviderIntegrationTest) requires a real GCP project — set GCP_PROJECT_ID env var to run
  • Sample app: follow samples/gcp/README.md to run end-to-end

mahpatil added 9 commits June 3, 2026 13:48
Signed-off-by: Mahesh Patil <maheshfinity@gmail.com>
Signed-off-by: Mahesh Patil <maheshfinity@gmail.com>
…til/java-sdk-contrib into feat/gcp-parameter-manager

Signed-off-by: Mahesh Patil <maheshfinity@gmail.com>
Signed-off-by: Mahesh Patil <maheshfinity@gmail.com>
1. add location id to options as gcp params require location
2. fixed parameter manager classes to use common gcp classes post refactor

Signed-off-by: Mahesh Patil <maheshfinity@gmail.com>
Signed-off-by: Mahesh Patil <maheshfinity@gmail.com>
Signed-off-by: Mahesh Patil <maheshfinity@gmail.com>
Signed-off-by: Mahesh Patil <maheshfinity@gmail.com>
Signed-off-by: Mahesh Patil <maheshfinity@gmail.com>
@mahpatil mahpatil requested a review from a team as a code owner June 5, 2026 18:21

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request introduces support for Google Cloud Parameter Manager by refactoring the existing GCP provider into a shared AbstractGcpProvider base class and adding the new GcpParameterManagerProvider. It also updates the sample applications, setup/teardown scripts, and tests to support both Secret Manager and Parameter Manager backends. Feedback on these changes highlights several issues: locationId in GcpProviderOptions needs a default value of "global" to prevent null pointer exceptions; the Parameter Manager setup script contains broken update and versioning logic; redundant log statements should be cleaned up; synchronizing on this during blocking network calls creates a performance bottleneck; and the teardown script's deletion chaining is risky and should be decoupled.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread samples/gcp/setup.sh
Comment thread samples/gcp/teardown.sh
@mahpatil mahpatil changed the title Feat/gcp parameter manager feat/gcp parameter manager Jun 6, 2026
@mahpatil mahpatil changed the title feat/gcp parameter manager feat(gcp): parameter manager Jun 6, 2026
@mahpatil mahpatil changed the title feat(gcp): parameter manager feat(gcp): add parameter manager support Jun 6, 2026
mahpatil and others added 9 commits June 6, 2026 07:41
Signed-off-by: Mahesh Patil <maheshfinity@gmail.com>
… of creating new versoin2. locationId default to prevent null pointers3. update references to stale GcpParameterManagerOptions

Signed-off-by: Mahesh Patil <maheshfinity@gmail.com>
…of this ffor performance reasons

Signed-off-by: Mahesh Patil <maheshfinity@gmail.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Signed-off-by: Mahesh Patil <17205424+mahpatil@users.noreply.github.com>
Signed-off-by: Mahesh Patil <maheshfinity@gmail.com>
…til/java-sdk-contrib into feat/gcp-parameter-manager

Signed-off-by: Mahesh Patil <maheshfinity@gmail.com>
Signed-off-by: Mahesh Patil <maheshfinity@gmail.com>
Signed-off-by: Mahesh Patil <maheshfinity@gmail.com>
@mahpatil

mahpatil commented Jun 6, 2026

Copy link
Copy Markdown
Contributor Author

@toddbaert @aepfli this now adds parameter manager support to gcp provider.

Signed-off-by: Todd Baert <todd.baert@dynatrace.com>
log.debug("Fetching from cache name '{}'", key);
return cached.get();
}
synchronized (cache) {

@toddbaert toddbaert Jun 9, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Two related things:

  1. cache is reassigned in initialize() (line 39) but used as the lock in fetchWithCache (line 105). Call initialize() twice and readers end up holding different locks. (FlagCache.get also locks internally, so the outer synchronized is partly redundant.)

  2. Nothing stops initialize() from being called twice, if I'm reading correctly... if it is, the first client just leaks. The convention elsewhere in this repo is to guard with an AtomicBoolean and throw GeneralError("already initialized").

Aadd the guard, and make cache final (or use a dedicated lock) to fix both... and maybe add a test. WDYT?

* @throws GeneralError for any other GCP API error
*/
private String fetchFromGcp(String secretName) {
protected String fetchFromGcp(String secretName) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The catch (Exception e) below drops the cause; sibling GcpParameterManagerProvider#fetchFromGcp keeps it. Let's stay consistent and preserve it (helps when debugging GCP errors).

@toddbaert

toddbaert commented Jun 9, 2026

Copy link
Copy Markdown
Member

Approving; pushed a small fixup to revert the manual version bumps (release-please owns those) and wire samples/gcp/pom.xml into the release-please config so the sample's dep version bumps automatically.

Also opened 2 recommendations... this one in particular I'd really like to see done before release (unless I'm missing something).

Once this is merged, I can merge the pending "release please" PR, which will release the package.

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