fix: GcpProjectIdProvider when using Secret Manager (#3664)

fixes #3181 , also fixes #3564

cherry-picked changes from #3383 to allow ci properly run. Adding tests.
@PatrickGotthard 's original comment on root cause: #3181 (comment)

When using Secret Manager (sm) module, expected behavior for GcpProjectIdProvider should be:
- in Spring Bootstrap Phase (where sm if configured), order of discovery: secretmanager.project-id >>  gcp.project-id >> default (according to DefaultGcpProjectIdProvider)
- in Application Context, GcpProjectIdProvider bean should behave similar to other modules, with no knowledge of secretmanager.project-id, thus: gcp.project-id >> default


Current behavior prior to this fix, the discovery order for both cases are:
- secretmanager.project-id >> default 

Fix is to let SecretManagerConfigDataLocationResolver recognizing GcpProperties and **not** promoting GcpProjectIdProvider to application context.

Author: zhumin8

docs/src/main/asciidoc/secretmanager.adoc

@@ -41,7 +41,7 @@ This can be overridden using the authentication properties.
 | `spring.cloud.gcp.secretmanager.enabled` | Enables the Secret Manager integration. | No | `true`
 | `spring.cloud.gcp.secretmanager.credentials.location` | OAuth2 credentials for authenticating to the Google Cloud Secret Manager API. | No | By default, infers credentials from https://cloud.google.com/docs/authentication/production[Application Default Credentials].
 | `spring.cloud.gcp.secretmanager.credentials.encoded-key` | Base64-encoded contents of OAuth2 account private key for authenticating to the Google Cloud Secret Manager API. | No | By default, infers credentials from https://cloud.google.com/docs/authentication/production[Application Default Credentials].
-| `spring.cloud.gcp.secretmanager.project-id` | The default Google Cloud project used to access Secret Manager API for the template and property source. | No | By default, infers the project from https://cloud.google.com/docs/authentication/production[Application Default Credentials].
+| `spring.cloud.gcp.secretmanager.project-id` | The default Google Cloud project used to access Secret Manager API for the template and property source. | No | Default to the one in the <<spring-cloud-gcp-core,Spring Framework on Google Cloud Core Module>>.
 |`spring.cloud.gcp.secretmanager.allow-default-secret`| Define the behavior when accessing a non-existent secret string/bytes. +
 If set to `true`, `null` will be returned when accessing a non-existent secret; otherwise throwing an exception. | No | `false`
 |===

spring-cloud-gcp-autoconfigure/src/main/java/com/google/cloud/spring/autoconfigure/core/GcpProperties.java

@@ -16,17 +16,22 @@
 
 package com.google.cloud.spring.autoconfigure.core;
 
+import static com.google.cloud.spring.autoconfigure.core.GcpProperties.CORE_PROPERTY_PREFIX;
+
 import com.google.cloud.spring.core.Credentials;
 import com.google.cloud.spring.core.CredentialsSupplier;
 import org.springframework.boot.context.properties.ConfigurationProperties;
 import org.springframework.boot.context.properties.NestedConfigurationProperty;
 import org.springframework.context.annotation.ImportRuntimeHints;
 
 /** Top-level auto-config settings. */
-@ConfigurationProperties("spring.cloud.gcp")
+@ConfigurationProperties(CORE_PROPERTY_PREFIX)
 @ImportRuntimeHints(CredentialsRuntimeHints.class)
 public class GcpProperties implements CredentialsSupplier {
 
+  /** Configuration prefix. */
+  public static final String CORE_PROPERTY_PREFIX = "spring.cloud.gcp";
+
   /** GCP project ID where services are running. */
   private String projectId;
 

spring-cloud-gcp-autoconfigure/src/main/java/com/google/cloud/spring/autoconfigure/secretmanager/GcpSecretManagerAutoConfiguration.java

@@ -19,11 +19,13 @@
 import com.google.api.gax.core.CredentialsProvider;
 import com.google.cloud.secretmanager.v1.SecretManagerServiceClient;
 import com.google.cloud.secretmanager.v1.SecretManagerServiceSettings;
+import com.google.cloud.spring.autoconfigure.core.GcpContextAutoConfiguration;
 import com.google.cloud.spring.core.GcpProjectIdProvider;
 import com.google.cloud.spring.core.UserAgentHeaderProvider;
 import com.google.cloud.spring.secretmanager.SecretManagerTemplate;
 import java.io.IOException;
 import org.springframework.boot.autoconfigure.AutoConfiguration;
+import org.springframework.boot.autoconfigure.AutoConfigureAfter;
 import org.springframework.boot.autoconfigure.condition.ConditionalOnClass;
 import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean;
 import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty;
@@ -39,6 +41,7 @@
 @EnableConfigurationProperties(GcpSecretManagerProperties.class)
 @ConditionalOnClass(SecretManagerTemplate.class)
 @ConditionalOnProperty(value = "spring.cloud.gcp.secretmanager.enabled", matchIfMissing = true)
+@AutoConfigureAfter(GcpContextAutoConfiguration.class)
 public class GcpSecretManagerAutoConfiguration {
 
   private final GcpProjectIdProvider gcpProjectIdProvider;

spring-cloud-gcp-autoconfigure/src/main/java/com/google/cloud/spring/autoconfigure/secretmanager/SecretManagerConfigDataLocationResolver.java

@@ -21,6 +21,7 @@
 
 import com.google.cloud.secretmanager.v1.SecretManagerServiceClient;
 import com.google.cloud.secretmanager.v1.SecretManagerServiceSettings;
+import com.google.cloud.spring.autoconfigure.core.GcpProperties;
 import com.google.cloud.spring.core.DefaultCredentialsProvider;
 import com.google.cloud.spring.core.DefaultGcpProjectIdProvider;
 import com.google.cloud.spring.core.GcpProjectIdProvider;
@@ -35,6 +36,7 @@
 import org.slf4j.LoggerFactory;
 import org.springframework.beans.factory.config.ConfigurableListableBeanFactory;
 import org.springframework.boot.BootstrapRegistry;
+import org.springframework.boot.ConfigurableBootstrapContext;
 import org.springframework.boot.context.config.ConfigDataLocation;
 import org.springframework.boot.context.config.ConfigDataLocationNotFoundException;
 import org.springframework.boot.context.config.ConfigDataLocationResolver;
@@ -70,6 +72,8 @@ public List<SecretManagerConfigDataResource> resolve(ConfigDataLocationResolverC
   }
 
   private static void registerSecretManagerBeans(ConfigDataLocationResolverContext context) {
+    // Register the Core properties.
+    registerBean(context, GcpProperties.class, getGcpProperties(context));
     // Register the Secret Manager properties.
     registerBean(
         context, GcpSecretManagerProperties.class, getSecretManagerProperties(context));
@@ -80,30 +84,41 @@ private static void registerSecretManagerBeans(ConfigDataLocationResolverContext
         // lazy register the client solely for unit test.
         BootstrapRegistry.InstanceSupplier.from(() -> createSecretManagerClient(context)));
     // Register the GCP Project ID provider.
-    registerAndPromoteBean(
-        context,
-        GcpProjectIdProvider.class,
-        BootstrapRegistry.InstanceSupplier.of(createProjectIdProvider(context)));
+    registerBean(context, GcpProjectIdProvider.class, createProjectIdProvider(context));
     // Register the Secret Manager template.
     registerAndPromoteBean(
         context,
         SecretManagerTemplate.class,
         BootstrapRegistry.InstanceSupplier.of(createSecretManagerTemplate(context)));
   }
 
+  private static GcpProperties getGcpProperties(ConfigDataLocationResolverContext context) {
+    return context
+        .getBinder()
+        .bind(GcpProperties.CORE_PROPERTY_PREFIX, GcpProperties.class)
+        .orElse(new GcpProperties());
+  }
+
   private static GcpSecretManagerProperties getSecretManagerProperties(
       ConfigDataLocationResolverContext context) {
     return context.getBinder()
         .bind(GcpSecretManagerProperties.PREFIX, GcpSecretManagerProperties.class)
         .orElse(new GcpSecretManagerProperties());
   }
 
-  private static GcpProjectIdProvider createProjectIdProvider(
-      ConfigDataLocationResolverContext context) {
-    GcpSecretManagerProperties properties = context.getBootstrapContext()
-        .get(GcpSecretManagerProperties.class);
-    return properties.getProjectId() != null
-        ? properties::getProjectId : new DefaultGcpProjectIdProvider();
+  @VisibleForTesting
+  static GcpProjectIdProvider createProjectIdProvider(ConfigDataLocationResolverContext context) {
+    ConfigurableBootstrapContext bootstrapContext = context.getBootstrapContext();
+    GcpSecretManagerProperties secretManagerProperties =
+        bootstrapContext.get(GcpSecretManagerProperties.class);
+    if (secretManagerProperties.getProjectId() != null) {
+      return secretManagerProperties::getProjectId;
+    }
+    GcpProperties gcpProperties = bootstrapContext.get(GcpProperties.class);
+    if (gcpProperties.getProjectId() != null) {
+      return gcpProperties::getProjectId;
+    }
+    return new DefaultGcpProjectIdProvider();
   }
 
   @VisibleForTesting

spring-cloud-gcp-autoconfigure/src/test/java/com/google/cloud/spring/autoconfigure/secretmanager/SecretManagerCompatibilityTests.java

@@ -13,7 +13,6 @@
 import com.google.protobuf.ByteString;
 import java.util.stream.Stream;
 import org.junit.jupiter.api.BeforeEach;
-import org.junit.jupiter.api.Test;
 import org.junit.jupiter.params.ParameterizedTest;
 import org.junit.jupiter.params.provider.Arguments;
 import org.junit.jupiter.params.provider.MethodSource;
@@ -34,17 +33,17 @@ class SecretManagerCompatibilityTests {
 
   static Stream<Arguments> prefixes() {
     return Stream.of(
-        Arguments.of("sm://"),
-        Arguments.of("sm@")
-    );
+        Arguments.of("sm://", "spring.cloud.gcp.project-id="),
+        Arguments.of("sm://", "spring.cloud.gcp.secretmanager.project-id="),
+        Arguments.of("sm@", "spring.cloud.gcp.project-id="),
+        Arguments.of("sm@", "spring.cloud.gcp.secretmanager.project-id="));
   }
 
   @BeforeEach
   void init() {
     application = new SpringApplicationBuilder(SecretManagerCompatibilityTests.class)
         .web(WebApplicationType.NONE)
         .properties(
-            "spring.cloud.gcp.secretmanager.project-id=" + PROJECT_NAME,
             "spring.cloud.gcp.sql.enabled=false");
 
     client = mock(SecretManagerServiceClient.class);
@@ -77,15 +76,13 @@ void init() {
    */
   @ParameterizedTest
   @MethodSource("prefixes")
-  void testConfigurationWhenDefaultSecretIsNotAllowed(String prefix) {
-    application.properties(
-            "spring.config.import=" + prefix)
+  void testConfigurationWhenDefaultSecretIsNotAllowed(String prefix, String projectIdPropertyName) {
+    application
+        .properties(projectIdPropertyName + PROJECT_NAME, "spring.config.import=" + prefix)
         .addBootstrapRegistryInitializer(
-            (registry) -> registry.registerIfAbsent(
-                SecretManagerServiceClient.class,
-                InstanceSupplier.of(client)
-            )
-        );
+            (registry) ->
+                registry.registerIfAbsent(
+                    SecretManagerServiceClient.class, InstanceSupplier.of(client)));
     try (ConfigurableApplicationContext applicationContext = application.run()) {
       ConfigurableEnvironment environment = applicationContext.getEnvironment();
       assertThat(environment.getProperty(prefix + "my-secret")).isEqualTo("newSecret");
@@ -96,16 +93,16 @@ void testConfigurationWhenDefaultSecretIsNotAllowed(String prefix) {
 
   @ParameterizedTest
   @MethodSource("prefixes")
-  void testConfigurationWhenDefaultSecretIsAllowed(String prefix) {
-    application.properties(
+  void testConfigurationWhenDefaultSecretIsAllowed(String prefix, String projectIdPropertyName) {
+    application
+        .properties(
+            projectIdPropertyName + PROJECT_NAME,
             "spring.cloud.gcp.secretmanager.allow-default-secret=true",
             "spring.config.import=" + prefix)
         .addBootstrapRegistryInitializer(
-            (registry) -> registry.registerIfAbsent(
-                SecretManagerServiceClient.class,
-                InstanceSupplier.of(client)
-            )
-        );
+            (registry) ->
+                registry.registerIfAbsent(
+                    SecretManagerServiceClient.class, InstanceSupplier.of(client)));
     try (ConfigurableApplicationContext applicationContext = application.run()) {
       ConfigurableEnvironment environment = applicationContext.getEnvironment();
       assertThat(environment.getProperty(prefix + "my-secret")).isEqualTo("newSecret");

spring-cloud-gcp-autoconfigure/src/test/java/com/google/cloud/spring/autoconfigure/secretmanager/SecretManagerConfigDataLocationResolverUnitTests.java

@@ -7,6 +7,9 @@
 
 import com.google.api.gax.core.CredentialsProvider;
 import com.google.cloud.secretmanager.v1.SecretManagerServiceClient;
+import com.google.cloud.spring.autoconfigure.core.GcpProperties;
+import com.google.cloud.spring.core.DefaultGcpProjectIdProvider;
+import com.google.cloud.spring.core.GcpProjectIdProvider;
 import java.util.List;
 import java.util.stream.Stream;
 import org.junit.jupiter.api.BeforeEach;
@@ -31,6 +34,8 @@ class SecretManagerConfigDataLocationResolverUnitTests {
   private final ConfigDataLocationResolverContext context = mock(
       ConfigDataLocationResolverContext.class);
   private final DefaultBootstrapContext defaultBootstrapContext = new DefaultBootstrapContext();
+  private final GcpSecretManagerProperties secretManagerProperties = mock(GcpSecretManagerProperties.class);
+  private final GcpProperties gcpProperties = mock(GcpProperties.class);
 
   static Stream<Arguments> prefixes() {
     return Stream.of(
@@ -76,12 +81,14 @@ void resolveReturnsConfigDataLocation(String prefix) {
 
   @BeforeEach
   void registerBean() {
-    GcpSecretManagerProperties properties = mock(GcpSecretManagerProperties.class);
     CredentialsProvider credentialsProvider = mock(CredentialsProvider.class);
     SecretManagerServiceClient secretManagerServiceClient = mock(SecretManagerServiceClient.class);
 
-    defaultBootstrapContext.register(GcpSecretManagerProperties.class,
-        BootstrapRegistry.InstanceSupplier.of(properties));
+    defaultBootstrapContext.register(
+        GcpProperties.class, BootstrapRegistry.InstanceSupplier.of(gcpProperties));
+    defaultBootstrapContext.register(
+        GcpSecretManagerProperties.class,
+        BootstrapRegistry.InstanceSupplier.of(secretManagerProperties));
     defaultBootstrapContext.register(CredentialsProvider.class,
         BootstrapRegistry.InstanceSupplier.of(credentialsProvider));
     defaultBootstrapContext.register(SecretManagerServiceClient.class,
@@ -90,4 +97,28 @@ void registerBean() {
     when(context.getBinder()).thenReturn(new Binder());
     when(context.getBootstrapContext()).thenReturn(defaultBootstrapContext);
   }
+
+  @Test
+  void testSecretManagerProjectIdTakesPrecedence() {
+    when(secretManagerProperties.getProjectId()).thenReturn("secret-manager-property-id");
+    when(gcpProperties.getProjectId()).thenReturn("gcp-project-id");
+    GcpProjectIdProvider projectIdProvider =
+        SecretManagerConfigDataLocationResolver.createProjectIdProvider(context);
+    assertThat(projectIdProvider.getProjectId()).isEqualTo("secret-manager-property-id");
+  }
+
+  @Test
+  void testProjectIdUseCoreWhenNoSecretManagerProjectId() {
+    when(gcpProperties.getProjectId()).thenReturn("gcp-project-id");
+    GcpProjectIdProvider projectIdProvider =
+        SecretManagerConfigDataLocationResolver.createProjectIdProvider(context);
+    assertThat(projectIdProvider.getProjectId()).isEqualTo("gcp-project-id");
+  }
+
+  @Test
+  void testProjectIdFallBackToDefault() {
+    GcpProjectIdProvider projectIdProvider =
+        SecretManagerConfigDataLocationResolver.createProjectIdProvider(context);
+    assertThat(projectIdProvider).isInstanceOf(DefaultGcpProjectIdProvider.class);
+  }
 }

spring-cloud-gcp-samples/spring-cloud-gcp-secretmanager-sample/src/test/java/com/example/SecretManagerSamplePropertyIdTest.java

@@ -0,0 +1,44 @@
+package com.example;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+import com.google.cloud.spring.core.GcpProjectIdProvider;
+import com.google.cloud.spring.secretmanager.SecretManagerTemplate;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.condition.EnabledIfSystemProperty;
+import org.junit.jupiter.api.extension.ExtendWith;
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.boot.test.context.SpringBootTest;
+import org.springframework.test.context.TestPropertySource;
+import org.springframework.test.context.aot.DisabledInAotMode;
+import org.springframework.test.context.junit.jupiter.SpringExtension;
+
+@EnabledIfSystemProperty(named = "it.secretmanager", matches = "true")
+@ExtendWith(SpringExtension.class)
+@SpringBootTest(
+    webEnvironment = SpringBootTest.WebEnvironment.RANDOM_PORT,
+    classes = SecretManagerApplication.class)
+@TestPropertySource(
+    properties = {
+      "spring.cloud.gcp.secretmanager.project-id=spring-cloud-gcp-ci",
+      "spring.cloud.gcp.project-id=gcp-project-id"
+    })
+@DisabledInAotMode
+class SecretManagerSamplePropertyIdTest {
+
+  @Autowired private SecretManagerTemplate secretManagerTemplate;
+
+  @Autowired private GcpProjectIdProvider gcpProjectIdProvider;
+
+  // This test verifies propertyId is correctly recognized.
+  // When secretmanager.project-id and project gcp.project-id are both set,
+  // secretManagerTemplate should recognize secretmanager.project-id
+  // but gcpProjectIdProvider should only capture gcp.project-id
+  @Test
+  void testProjectIdCorrect() {
+    String projectId = secretManagerTemplate.getProjectId();
+    assertThat(projectId).isEqualTo("spring-cloud-gcp-ci");
+    String gcpProjectId = gcpProjectIdProvider.getProjectId();
+    assertThat(gcpProjectId).isEqualTo("gcp-project-id");
+  }
+}