SecretManager autoconfiguration ignores and superseeds spring.cloud.gcp.project-id #3181

mieseprem · 2024-08-30T11:33:59Z

This issue is regarding the projectId value returned by GcpProjectIdProvider when SecretManager is enabled.

I know, as an example, theses two closed issues, but I think they doesn't cover the root cause:

Regarding the documentation, the GcpProjectIdProvider.getProjectId() method will always return the value of the property spring.cloud.gcp.project-id if set. There is no limitation regarding the use of SecretManager.

In addition, in the SecretManager section of the documentation the project-id is (if not set as property): "By default, infers the project from Application Default Credentials"

I wasn't able to to determine the method who creates the GcpProjectIdProvider bean for the SecretManagerAutoConfiguration, but it definitivley get its values not from the spring.cloud.gcp.project-id property.
Because of this presence of a GcpProjectIdProvider bean there is no propper one created in GcpContextAutoConfiguration.

I created a simple repository to demonstrate the issue: https://github.com/mieseprem/projectIdIssue
Please keep in mind that this issue is not on how to access secrets or other services in GCP. This is only about how the project-id value is determined and provided to all beans that relay on the GcpProjectIdProvider

In my example project, there is a profile named "xxx" and the project-id is set to a specific value. If you run the application with active profile "xxx" you will see 2 things:

the GcpProjectIdProvider is created via 'GcpContextAutoConfiguration'
the project-id value is the one set as spring.cloud.gcp.project-id

...
2024-08-30T11:08:56.545+02:00  INFO 99194 --- [demo] [           main] com.example.demo.DemoApplication         : The following 1 profile is active: "xxx"
...
2024-08-30T11:08:56.680+02:00  INFO 99194 --- [demo] [           main] c.g.c.s.a.c.GcpContextAutoConfiguration  : The default project ID is my-project-id
...
2024-08-30T11:08:57.237+02:00  INFO 99194 --- [demo] [           main] com.example.demo.DemoApplication         : Started DemoApplication in 0.813 seconds (process running for 1.003)
Project ID: my-project-id
...

If you now enable the Config Data API to get Secret value via "sm://" prefix, the GcpProjectIdProvider is created in a different way and spring.cloud.gcp.project-id is not used anymore.
(Please uncomment these lines in application.yml and run again with "xxx" profile)

You'll see, the GcpProjectIdProvider is not created via GcpContextAutoConfiguration but - strong guess - determined from this places:

The project ID specified by the GOOGLE_CLOUD_PROJECT environment variable
The Google App Engine project ID
The project ID specified in the JSON credentials file pointed by the GOOGLE_APPLICATION_CREDENTIALS environment variable
The Google Cloud SDK project ID
The Google Compute Engine project ID, from the Google Compute Engine Metadata Server

If none of them is present/configured this will happen:

...
2024-08-30T12:07:52.190+02:00  INFO 421 --- [demo] [           main] com.example.demo.DemoApplication         : The following 1 profile is active: "xxx"
...
2024-08-30T12:07:52.382+02:00  INFO 421 --- [demo] [           main] com.example.demo.DemoApplication         : Started DemoApplication in 1.151 seconds (process running for 1.403)
Project ID: null
...

Note: even if I move spring.cloud.gcp.project-id into default profile it is ignored as of activating spring.cloud.import

As a workarount we can provide the project-id via spring.cloud.gcp.secretmanager.project-id property.
(Please uncomment these lines in application.yml
As a result you'll see the project-id isn't null anymore, but the variable is not resolved:

...
2024-08-30T12:15:08.602+02:00  INFO 589 --- [demo] [           main] com.example.demo.DemoApplication         : The following 1 profile is active: "xxx"
...
2024-08-30T12:15:08.800+02:00  INFO 589 --- [demo] [           main] com.example.demo.DemoApplication         : Started DemoApplication in 1.034 seconds (process running for 1.383)
Project ID: ${spring.cloud.gcp.project-id}
...

Btw. the spring.cloud.gcp.secretmanager.project-id property can't be set in any profile (only in default). If set in any profile it is ignored.

Now, why do I think this is a bug (or at least not the expected behaviour)?

I think it's not okay that the SecretManager auto configuration disturbes all other GCP beans from being configured propperly.

if spring.cloud.gcp.project-id is set it should be used
setting spring.cloud.gcp.secretmanager.project-id shoul'd change the ProjectIdProvider for all GCP components
if spring.cloud.gcp.secretmanager.project-id can't be set in a profile or can't use a property value from a profile there is no way to configure a value based on a profile from within springboot. I must provide the value from outside via paramater, env var, or from the list mentioned further up.

The text was updated successfully, but these errors were encountered:

zhumin8 · 2024-10-22T21:56:34Z

On first pass, this looks like a bug to me. I will take another look when I can.
A few things:

I wasn't able to to determine the method who creates the GcpProjectIdProvider bean for the SecretManagerAutoConfiguration

Generally speaking, we expect project-id set by module property to override the one set across modules via GcpContextAutoConfiguration. That said, I run your sample code and agree it seems weird when turning on config. I will revisit this.

In addition, in the SecretManager section of the documentation the project-id is (if not set as property): "By default, infers the project from Application Default Credentials"

This is likely a bug/typo in the documentation. Will double check on this.

PatrickGotthard · 2024-11-15T12:12:39Z

I have the same issue and analyzed the problem. There are two places where the GcpProjectIdProvider is registered:

The problem occurs because the second method will be executed before the GcpContextAutoConfiguration and registers

a GcpProjectIdProvider that returns spring.cloud.gcp.secretmanager.project-id (if present)
or a DefaultGcpProjectIdProvider that does not look at the properties at all (internally calls a method from the official Google Cloud SDK)

The SecretManagerConfigDataLocationResolver must not register a GcpProjectIdProvider otherwise it will be used across all modules.

PatrickGotthard · 2024-11-16T11:42:47Z

I've possibly fixed the issue and opened the following PR: https://github.com/GoogleCloudPlatform/spring-cloud-gcp/pull/3383/files

mieseprem · 2024-11-20T10:42:33Z

Hi @PatrickGotthard , I build your repository locally and tried to use your version in my example code.
To be honest, I already failed building your code only because of checkstyle violations. I never worked with checkstyle so I don't know how fo autofix it (I always use Spotless and there it's just a spotlessApply).

However, my plan to verify if your changes fixes the problem was to:

mvn install your branch with a specific version set in **/pom.xml (I used 5.8.99)

In my example project

add mavenLocal() as repository and replace springboot-starter with dedicated dependencies:

...
repositories {
    mavenLocal()
    mavenCentral()
}

dependencies {
    implementation("com.google.cloud:spring-cloud-gcp-starter")
        // implementation("com.google.cloud:spring-cloud-gcp-starter-secretmanager")
    implementation("com.google.cloud:spring-cloud-gcp-core:5.8.99")
    implementation("com.google.cloud:spring-cloud-gcp-autoconfigure:5.8.99")
    implementation("com.google.cloud:spring-cloud-gcp-secretmanager:5.8.99")
    implementation("org.springframework.cloud:spring-cloud-context")
}
...

do all steps to reproduce the issues

If you could give me a hint on how to get away the checkstyle issues, I could continue with testing.

~/git/spring-cloud-gcp [fix-secretmanager-projectid|✚ 207] 
11:36 $ ./mvnw install
[INFO] Scanning for projects...
...
[INFO] --- checkstyle:3.5.0:check (validate-google-style) @ spring-cloud-gcp-autoconfigure ---
[WARNING] src/main/java/com/google/cloud/spring/autoconfigure/secretmanager/SecretManagerConfigDataLocationResolver.java:[111,5] (whitespace) WhitespaceAfter: 'if' is not followed by whitespace.
[WARNING] src/main/java/com/google/cloud/spring/autoconfigure/secretmanager/SecretManagerConfigDataLocationResolver.java:[111,5] (whitespace) WhitespaceAround: WhitespaceAround: 'if' is not followed by whitespace. Empty blocks may only be represented as {} when not part of a multi-block statement (4.1.3)
[WARNING] src/main/java/com/google/cloud/spring/autoconfigure/secretmanager/SecretManagerConfigDataLocationResolver.java:[112,9] (indentation) Indentation: 'if' child has incorrect indentation level 8, expected level should be 6.
[WARNING] src/main/java/com/google/cloud/spring/autoconfigure/secretmanager/SecretManagerConfigDataLocationResolver.java:[116,5] (whitespace) WhitespaceAfter: 'if' is not followed by whitespace.
[WARNING] src/main/java/com/google/cloud/spring/autoconfigure/secretmanager/SecretManagerConfigDataLocationResolver.java:[116,5] (whitespace) WhitespaceAround: WhitespaceAround: 'if' is not followed by whitespace. Empty blocks may only be represented as {} when not part of a multi-block statement (4.1.3)
[WARNING] src/main/java/com/google/cloud/spring/autoconfigure/secretmanager/SecretManagerConfigDataLocationResolver.java:[117,9] (indentation) Indentation: 'if' child has incorrect indentation level 8, expected level should be 6.
...
[INFO] Spring Framework on Google Cloud Module - Autoconfigure 5.8.99 FAILURE [  4.227 s]
...
[INFO] ------------------------------------------------------------------------
[INFO] BUILD FAILURE
[INFO] ------------------------------------------------------------------------
[INFO] Total time:  56.167 s
[INFO] Finished at: 2024-11-20T11:36:26+01:00
[INFO] ------------------------------------------------------------------------
[ERROR] Failed to execute goal org.apache.maven.plugins:maven-checkstyle-plugin:3.5.0:check (validate-google-style) on project spring-cloud-gcp-autoconfigure: You have 6 Checkstyle violations. -> [Help 1]

Anyway, I will try now with adding the whitespaces manually....

PatrickGotthard · 2024-11-20T11:03:11Z

Hi @mieseprem,

I wasn't able to fix the issue on the main branch so I checked out the latest release tag and opened only some of the modules in Eclipse. Eclipse then uses the code of the open projects instead of the downloaded artifacts.

Regards,
Patrick

mieseprem · 2024-11-20T12:52:56Z

To put it right away, I am not a code owner here and cannot approve or merge. I am just the creator of the issue.

Regarding your fix I can verify two things:

spring.cloud.gcp.project-id is now used for GcpProjectIdProvider regardless of having Config Data API enabled or not
configure spring.cloud.gcp.secretmanager.project-id doesn't overwrite spring.cloud.gcp.project-id anymore

There is still one thing left (but that is not part of your PR):

if spring.cloud.gcp.secretmanager.project-id can't be set in a profile or can't use a property value from a profile there is no way to configure a value based on a profile from within springboot. I must provide the value from outside via paramater, env var, or from the list mentioned further up.

But, this point is nothing I challenge at moment. So, in my opinion, it's up to Google if they want to have it handled or keep it as it is.

Regarding your PR, I'm very fine with integrating the code here. But to be compliant with Checkstyle, you should add the following to your branch:

Subject: [PATCH] Add whitespaces to comply with Checkstyle rules
---
Index: spring-cloud-gcp-autoconfigure/src/main/java/com/google/cloud/spring/autoconfigure/secretmanager/SecretManagerConfigDataLocationResolver.java
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/spring-cloud-gcp-autoconfigure/src/main/java/com/google/cloud/spring/autoconfigure/secretmanager/SecretManagerConfigDataLocationResolver.java b/spring-cloud-gcp-autoconfigure/src/main/java/com/google/cloud/spring/autoconfigure/secretmanager/SecretManagerConfigDataLocationResolver.java
--- a/spring-cloud-gcp-autoconfigure/src/main/java/com/google/cloud/spring/autoconfigure/secretmanager/SecretManagerConfigDataLocationResolver.java	(revision 1bb45e4ff9986e9a5009fe4db69dbb3eae8632ae)
+++ b/spring-cloud-gcp-autoconfigure/src/main/java/com/google/cloud/spring/autoconfigure/secretmanager/SecretManagerConfigDataLocationResolver.java	(date 1732099490028)
@@ -108,13 +108,13 @@
     ConfigurableBootstrapContext bootstrapContext = context.getBootstrapContext();
     
     GcpSecretManagerProperties secretManagerProperties = bootstrapContext.get(GcpSecretManagerProperties.class);
-    if(secretManagerProperties.getProjectId() != null) {
-        return secretManagerProperties::getProjectId;
+    if (secretManagerProperties.getProjectId() != null) {
+      return secretManagerProperties::getProjectId;
     }
     
     GcpProperties gcpProperties = bootstrapContext.get(GcpProperties.class);
-    if(gcpProperties.getProjectId() != null) {
-        return gcpProperties::getProjectId;
+    if (gcpProperties.getProjectId() != null) {
+      return gcpProperties::getProjectId;
     }
     
     return new DefaultGcpProjectIdProvider();

Eden90 · 2024-11-20T20:09:10Z

To put it right away, I am not a code owner here and cannot approve or merge. I am just the creator of the issue.

Regarding your fix I can verify two things:

spring.cloud.gcp.project-id is now used for GcpProjectIdProvider regardless of having Config Data API enabled or not

configure spring.cloud.gcp.secretmanager.project-id doesn't overwrite spring.cloud.gcp.project-id anymore

There is still one thing left (but that is not part of your PR):

if spring.cloud.gcp.secretmanager.project-id can't be set in a profile or can't use a property value from a profile there is no way to configure a value based on a profile from within springboot. I must provide the value from outside via paramater, env var, or from the list mentioned further up.

But, this point is nothing I challenge at moment. So, in my opinion, it's up to Google if they want to have it handled or keep it as it is.

Regarding your PR, I'm very fine with integrating the code here. But to be compliant with Checkstyle, you should add the following to your branch:
Subject: [PATCH] Add whitespaces to comply with Checkstyle rules
---
Index: spring-cloud-gcp-autoconfigure/src/main/java/com/google/cloud/spring/autoconfigure/secretmanager/SecretManagerConfigDataLocationResolver.java
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/spring-cloud-gcp-autoconfigure/src/main/java/com/google/cloud/spring/autoconfigure/secretmanager/SecretManagerConfigDataLocationResolver.java b/spring-cloud-gcp-autoconfigure/src/main/java/com/google/cloud/spring/autoconfigure/secretmanager/SecretManagerConfigDataLocationResolver.java
--- a/spring-cloud-gcp-autoconfigure/src/main/java/com/google/cloud/spring/autoconfigure/secretmanager/SecretManagerConfigDataLocationResolver.java	(revision 1bb45e4ff9986e9a5009fe4db69dbb3eae8632ae)
+++ b/spring-cloud-gcp-autoconfigure/src/main/java/com/google/cloud/spring/autoconfigure/secretmanager/SecretManagerConfigDataLocationResolver.java	(date 1732099490028)
@@ -108,13 +108,13 @@
     ConfigurableBootstrapContext bootstrapContext = context.getBootstrapContext();
     
     GcpSecretManagerProperties secretManagerProperties = bootstrapContext.get(GcpSecretManagerProperties.class);
-    if(secretManagerProperties.getProjectId() != null) {
-        return secretManagerProperties::getProjectId;
+    if (secretManagerProperties.getProjectId() != null) {
+      return secretManagerProperties::getProjectId;
     }
     
     GcpProperties gcpProperties = bootstrapContext.get(GcpProperties.class);
-    if(gcpProperties.getProjectId() != null) {
-        return gcpProperties::getProjectId;
+    if (gcpProperties.getProjectId() != null) {
+      return gcpProperties::getProjectId;
     }
     
     return new DefaultGcpProjectIdProvider();

Hi @PatrickGotthard , I build your repository locally and tried to use your version in my example code. To be honest, I already failed building your code only because of checkstyle violations. I never worked with checkstyle so I don't know how fo autofix it (I always use Spotless and there it's just a spotlessApply).

However, my plan to verify if your changes fixes the problem was to:

mvn install your branch with a specific version set in **/pom.xml (I used 5.8.99)

In my example project

add mavenLocal() as repository and replace springboot-starter with dedicated dependencies:

...
repositories {
    mavenLocal()
    mavenCentral()
}

dependencies {
    implementation("com.google.cloud:spring-cloud-gcp-starter")
        // implementation("com.google.cloud:spring-cloud-gcp-starter-secretmanager")
    implementation("com.google.cloud:spring-cloud-gcp-core:5.8.99")
    implementation("com.google.cloud:spring-cloud-gcp-autoconfigure:5.8.99")
    implementation("com.google.cloud:spring-cloud-gcp-secretmanager:5.8.99")
    implementation("org.springframework.cloud:spring-cloud-context")
}
...

do all steps to reproduce the issues

If you could give me a hint on how to get away the checkstyle issues, I could continue with testing.

~/git/spring-cloud-gcp [fix-secretmanager-projectid|✚ 207] 
11:36 $ ./mvnw install
[INFO] Scanning for projects...
...
[INFO] --- checkstyle:3.5.0:check (validate-google-style) @ spring-cloud-gcp-autoconfigure ---
[WARNING] src/main/java/com/google/cloud/spring/autoconfigure/secretmanager/SecretManagerConfigDataLocationResolver.java:[111,5] (whitespace) WhitespaceAfter: 'if' is not followed by whitespace.
[WARNING] src/main/java/com/google/cloud/spring/autoconfigure/secretmanager/SecretManagerConfigDataLocationResolver.java:[111,5] (whitespace) WhitespaceAround: WhitespaceAround: 'if' is not followed by whitespace. Empty blocks may only be represented as {} when not part of a multi-block statement (4.1.3)
[WARNING] src/main/java/com/google/cloud/spring/autoconfigure/secretmanager/SecretManagerConfigDataLocationResolver.java:[112,9] (indentation) Indentation: 'if' child has incorrect indentation level 8, expected level should be 6.
[WARNING] src/main/java/com/google/cloud/spring/autoconfigure/secretmanager/SecretManagerConfigDataLocationResolver.java:[116,5] (whitespace) WhitespaceAfter: 'if' is not followed by whitespace.
[WARNING] src/main/java/com/google/cloud/spring/autoconfigure/secretmanager/SecretManagerConfigDataLocationResolver.java:[116,5] (whitespace) WhitespaceAround: WhitespaceAround: 'if' is not followed by whitespace. Empty blocks may only be represented as {} when not part of a multi-block statement (4.1.3)
[WARNING] src/main/java/com/google/cloud/spring/autoconfigure/secretmanager/SecretManagerConfigDataLocationResolver.java:[117,9] (indentation) Indentation: 'if' child has incorrect indentation level 8, expected level should be 6.
...
[INFO] Spring Framework on Google Cloud Module - Autoconfigure 5.8.99 FAILURE [  4.227 s]
...
[INFO] ------------------------------------------------------------------------
[INFO] BUILD FAILURE
[INFO] ------------------------------------------------------------------------
[INFO] Total time:  56.167 s
[INFO] Finished at: 2024-11-20T11:36:26+01:00
[INFO] ------------------------------------------------------------------------
[ERROR] Failed to execute goal org.apache.maven.plugins:maven-checkstyle-plugin:3.5.0:check (validate-google-style) on project spring-cloud-gcp-autoconfigure: You have 6 Checkstyle violations. -> [Help 1]

Anyway, I will try now with adding the whitespaces manually....

Hi @mieseprem, you can just add -Dcheckstyle.skip property for your build e.g. ./mvnw clean install -DskipTests -Dcheckstyle.skip.

mieseprem · 2024-11-21T08:53:25Z

you can just add -Dcheckstyle.skip property for your build e.g. ./mvnw clean install -DskipTests -Dcheckstyle.skip

Thank you for your hint! 👍

mieseprem · 2024-11-27T14:30:04Z

Hello @zhumin8 , did you had a chance to have a look on Patrick's PR #3383 ?
In my opinion it looks good.

Eden90 · 2024-11-28T20:51:24Z

if spring.cloud.gcp.secretmanager.project-id can't be set in a profile or can't use a property value from a profile there is no way to configure a value based on a profile from within springboot. I must provide the value from outside via paramater, env var, or from the list mentioned further up.

As I understand it still doesn't fix the issue which I'm facing in my project. For me the issue still exists even with Patrick's PR.

mpeddada1 added type: bug Something isn't working priority: p2 labels Sep 5, 2024

burkedavison assigned zhumin8 Oct 3, 2024

PatrickGotthard mentioned this issue Nov 16, 2024

Fix wrong GcpProjectIdProvider when using Secret Manager #3383

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

SecretManager autoconfiguration ignores and superseeds spring.cloud.gcp.project-id #3181

SecretManager autoconfiguration ignores and superseeds spring.cloud.gcp.project-id #3181

mieseprem commented Aug 30, 2024 •

edited

Loading

zhumin8 commented Oct 22, 2024

PatrickGotthard commented Nov 15, 2024 •

edited

Loading

PatrickGotthard commented Nov 16, 2024 •

edited

Loading

mieseprem commented Nov 20, 2024 •

edited

Loading

PatrickGotthard commented Nov 20, 2024 •

edited

Loading

mieseprem commented Nov 20, 2024

Eden90 commented Nov 20, 2024

mieseprem commented Nov 21, 2024

mieseprem commented Nov 27, 2024

Eden90 commented Nov 28, 2024

SecretManager autoconfiguration ignores and superseeds spring.cloud.gcp.project-id #3181

SecretManager autoconfiguration ignores and superseeds spring.cloud.gcp.project-id #3181

Comments

mieseprem commented Aug 30, 2024 • edited Loading

zhumin8 commented Oct 22, 2024

PatrickGotthard commented Nov 15, 2024 • edited Loading

PatrickGotthard commented Nov 16, 2024 • edited Loading

mieseprem commented Nov 20, 2024 • edited Loading

PatrickGotthard commented Nov 20, 2024 • edited Loading

mieseprem commented Nov 20, 2024

Eden90 commented Nov 20, 2024

mieseprem commented Nov 21, 2024

mieseprem commented Nov 27, 2024

Eden90 commented Nov 28, 2024

mieseprem commented Aug 30, 2024 •

edited

Loading

PatrickGotthard commented Nov 15, 2024 •

edited

Loading

PatrickGotthard commented Nov 16, 2024 •

edited

Loading

mieseprem commented Nov 20, 2024 •

edited

Loading

PatrickGotthard commented Nov 20, 2024 •

edited

Loading