No longer possible to use secret-manager injected secret in @ConfigMapping class #491

jaworskib · 2023-09-02T09:53:47Z

I have a simple application with the following setup:

quarkus:
  google:
    cloud:
      project-id: ${GOOGLE_CLOUD_PROJECT}

# omitted irrelevant configurations

"%secret-manager":
  abc:
    xyz:
      keys:
        - id: some-key-id
          key: ${sm//some-key}
        - id: some-other-key-id
          key: ${sm//some-other-key}
        # etc ...

and a following @ConfigMapping interface:

@ConfigMapping(prefix = "abc.xyz")
public interface AbcXyzProperties {

    // other fields ...

    @NotEmpty
    List<Key> keys();

    public interface Key {
        String id();
        String key();
    }
}

Everything was working fine in the version 2.2.0 but since version 2.3.0 application startup ends with the following error:

2023-09-02 11:11:23,132 ERROR [io.qua.run.Application] (Quarkus Main Thread) Failed to start application (with profile [secret-manager]): java.lang.RuntimeException: Failed to start quarkus
	at io.quarkus.runner.ApplicationImpl.doStart(Unknown Source)
	at io.quarkus.runtime.Application.start(Application.java:101)
	at io.quarkus.runtime.ApplicationLifecycleManager.run(ApplicationLifecycleManager.java:111)
	at io.quarkus.runtime.Quarkus.run(Quarkus.java:71)
	at io.quarkus.runtime.Quarkus.run(Quarkus.java:44)
	at io.quarkus.runtime.Quarkus.run(Quarkus.java:124)
	at com.vodeno.dragon.Main.main(Main.java:15)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:77)
	at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.base/java.lang.reflect.Method.invoke(Method.java:568)
	at io.quarkus.runner.bootstrap.StartupActionImpl$1.run(StartupActionImpl.java:104)
	at java.base/java.lang.Thread.run(Thread.java:833)
Caused by: io.smallrye.config.ConfigValidationException: Configuration validation failed:
	java.lang.RuntimeException: Error injecting io.quarkiverse.googlecloudservices.common.GcpConfigHolder io.quarkiverse.googlecloudservices.secretmanager.runtime.SecretManagerProducer.gcpConfigHolder
        java.lang.RuntimeException: Error injecting io.quarkiverse.googlecloudservices.common.GcpConfigHolder io.quarkiverse.googlecloudservices.secretmanager.runtime.SecretManagerProducer.gcpConfigHolder
	at io.smallrye.config.ConfigMappingProvider.mapConfigurationInternal(ConfigMappingProvider.java:1003)
	at io.smallrye.config.ConfigMappingProvider.lambda$mapConfiguration$3(ConfigMappingProvider.java:959)
	at io.smallrye.config.SecretKeys.doUnlocked(SecretKeys.java:28)
	at io.smallrye.config.ConfigMappingProvider.mapConfiguration(ConfigMappingProvider.java:959)
	at io.smallrye.config.ConfigMappings.mapConfiguration(ConfigMappings.java:91)
	at io.smallrye.config.SmallRyeConfigBuilder.build(SmallRyeConfigBuilder.java:647)
	at io.quarkus.runtime.generated.Config.readConfig(Unknown Source)
	at io.quarkus.runtime.generated.Config.createRunTimeConfig(Unknown Source)
	at io.quarkus.deployment.steps.RuntimeConfigSetup.deploy(Unknown Source)
	... 13 more

which is a result of

  java.util.NoSuchElementException: SRCFG00027: Could not find a mapping for io.quarkiverse.googlecloudservices.common.GcpBootstrapConfiguration
  	at io.smallrye.config.ConfigMappings.getConfigMapping(ConfigMappings.java:111)
  	at io.smallrye.config.SmallRyeConfig.getConfigMapping(SmallRyeConfig.java:412)
  	at io.quarkus.arc.runtime.ConfigMappingCreator.create(ConfigMappingCreator.java:30)
  	at io.quarkiverse.googlecloudservices.common.GcpBootstrapConfiguration_a3a2a72d1f7f6e52c8a965e44841c94f0dd0f329_Synthetic_Bean.createSynthetic(Unknown Source)
  	at io.quarkiverse.googlecloudservices.common.GcpBootstrapConfiguration_a3a2a72d1f7f6e52c8a965e44841c94f0dd0f329_Synthetic_Bean.doCreate(Unknown Source)
  	at io.quarkiverse.googlecloudservices.common.GcpBootstrapConfiguration_a3a2a72d1f7f6e52c8a965e44841c94f0dd0f329_Synthetic_Bean.create(Unknown Source)
  	at io.quarkiverse.googlecloudservices.common.GcpBootstrapConfiguration_a3a2a72d1f7f6e52c8a965e44841c94f0dd0f329_Synthetic_Bean.get(Unknown Source)
  	at io.quarkiverse.googlecloudservices.common.GcpBootstrapConfiguration_a3a2a72d1f7f6e52c8a965e44841c94f0dd0f329_Synthetic_Bean.get(Unknown Source)
  	at io.quarkus.arc.impl.CurrentInjectionPointProvider.get(CurrentInjectionPointProvider.java:48)
  	at io.quarkiverse.googlecloudservices.common.GcpConfigHolder_Bean.doCreate(Unknown Source)
  	at io.quarkiverse.googlecloudservices.common.GcpConfigHolder_Bean.create(Unknown Source)
  	at io.quarkiverse.googlecloudservices.common.GcpConfigHolder_Bean.create(Unknown Source)
  	at io.quarkus.arc.impl.AbstractSharedContext.createInstanceHandle(AbstractSharedContext.java:113)
  	at io.quarkus.arc.impl.AbstractSharedContext$1.get(AbstractSharedContext.java:37)
  	at io.quarkus.arc.impl.AbstractSharedContext$1.get(AbstractSharedContext.java:34)
  	at io.quarkus.arc.impl.LazyValue.get(LazyValue.java:32)
  	at io.quarkus.arc.impl.ComputingCache.computeIfAbsent(ComputingCache.java:69)
  	at io.quarkus.arc.impl.AbstractSharedContext.get(AbstractSharedContext.java:34)
  	at io.quarkiverse.googlecloudservices.common.GcpConfigHolder_Bean.get(Unknown Source)
  	at io.quarkiverse.googlecloudservices.common.GcpConfigHolder_Bean.get(Unknown Source)
  	at io.quarkiverse.googlecloudservices.secretmanager.runtime.SecretManagerProducer_Bean.doCreate(Unknown Source)
  	at io.quarkiverse.googlecloudservices.secretmanager.runtime.SecretManagerProducer_Bean.create(Unknown Source)
  	at io.quarkiverse.googlecloudservices.secretmanager.runtime.SecretManagerProducer_Bean.create(Unknown Source)
  	at io.quarkus.arc.impl.AbstractSharedContext.createInstanceHandle(AbstractSharedContext.java:113)
  	at io.quarkus.arc.impl.AbstractSharedContext$1.get(AbstractSharedContext.java:37)
  	at io.quarkus.arc.impl.AbstractSharedContext$1.get(AbstractSharedContext.java:34)
  	at io.quarkus.arc.impl.LazyValue.get(LazyValue.java:32)
  	at io.quarkus.arc.impl.ComputingCache.computeIfAbsent(ComputingCache.java:69)
  	at io.quarkus.arc.impl.AbstractSharedContext.get(AbstractSharedContext.java:34)
  	at io.quarkiverse.googlecloudservices.secretmanager.runtime.SecretManagerProducer_Bean.get(Unknown Source)
  	at io.quarkiverse.googlecloudservices.secretmanager.runtime.SecretManagerProducer_Bean.get(Unknown Source)
  	at io.quarkiverse.googlecloudservices.secretmanager.runtime.SecretManagerProducer_ProducerMethod_secretManagerClient_da0d037e46ea741b638dc55997e8c2b5dd43f779_Bean.doCreate(Unknown Source)
  	at io.quarkiverse.googlecloudservices.secretmanager.runtime.SecretManagerProducer_ProducerMethod_secretManagerClient_da0d037e46ea741b638dc55997e8c2b5dd43f779_Bean.create(Unknown Source)
  	at io.quarkiverse.googlecloudservices.secretmanager.runtime.SecretManagerProducer_ProducerMethod_secretManagerClient_da0d037e46ea741b638dc55997e8c2b5dd43f779_Bean.create(Unknown Source)
  	at io.quarkus.arc.impl.AbstractSharedContext.createInstanceHandle(AbstractSharedContext.java:113)
  	at io.quarkus.arc.impl.AbstractSharedContext$1.get(AbstractSharedContext.java:37)
  	at io.quarkus.arc.impl.AbstractSharedContext$1.get(AbstractSharedContext.java:34)
  	at io.quarkus.arc.impl.LazyValue.get(LazyValue.java:32)
  	at io.quarkus.arc.impl.ComputingCache.computeIfAbsent(ComputingCache.java:69)
  	at io.quarkus.arc.impl.AbstractSharedContext.get(AbstractSharedContext.java:34)
  	at io.quarkiverse.googlecloudservices.secretmanager.runtime.SecretManagerProducer_ProducerMethod_secretManagerClient_da0d037e46ea741b638dc55997e8c2b5dd43f779_Bean.get(Unknown Source)
  	at io.quarkiverse.googlecloudservices.secretmanager.runtime.SecretManagerProducer_ProducerMethod_secretManagerClient_da0d037e46ea741b638dc55997e8c2b5dd43f779_Bean.get(Unknown Source)
  	at io.quarkus.arc.impl.ArcContainerImpl.beanInstanceHandle(ArcContainerImpl.java:557)
  	at io.quarkus.arc.impl.ArcContainerImpl.beanInstanceHandle(ArcContainerImpl.java:537)
  	at io.quarkus.arc.impl.ArcContainerImpl.beanInstanceHandle(ArcContainerImpl.java:570)
  	at io.quarkus.arc.impl.ArcContainerImpl.instanceHandle(ArcContainerImpl.java:532)
  	at io.quarkus.arc.impl.ArcContainerImpl.instance(ArcContainerImpl.java:279)
  	at io.quarkiverse.googlecloudservices.secretmanager.runtime.config.SecretManagerClientProvider.get(SecretManagerClientProvider.java:18)
  	at io.quarkiverse.googlecloudservices.secretmanager.runtime.config.SecretManagerConfigSource.getValue(SecretManagerConfigSource.java:46)
  	at io.smallrye.config.ConfigValueConfigSourceWrapper.getConfigValue(ConfigValueConfigSourceWrapper.java:20)
         ...

It seems like a direct cause of this error was a ConfigPhase change for GcpBootstrapConfiguration.
Do you plan to still support something like described above? Is there any known workaround for my case?

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

loicmathieu · 2023-09-05T11:07:50Z

@radcortez with the new way to load config source, it is no longer possible to inject config from the Google Cloud Secret Manager inside a config mapping. Is it a known limitation? Is there a fix or a workaround?

radcortez · 2023-09-05T11:46:05Z

Configs loaded by the Factories are available in the mappings phase, but looking into the stacktrace, there shouldn't be a call to io.quarkiverse.googlecloudservices.common.GcpConfigHolder during configuration init.

@jaworskib can you please provide a small reproducer so I can look into it? Thanks!

jaworskib · 2023-09-05T13:43:45Z

@radcortez sure, here you have a small reproducer: https://github.com/jaworskib/secret-manager-config-mapping
Unfortunately I cannot provide you GCP project id with secret manager access.

Thanks for looking into it!

radcortez · 2023-09-06T15:29:04Z

Thanks. I'll have a look.

radcortez · 2023-09-06T20:45:36Z

@jaworskib any chance to try out #493?

jaworskib · 2023-09-06T23:15:04Z

I'll try it tomorrow, but to be honest I'm not sure about the approach.

It seems like this approach requires adding some additional permissions to SA to be able to list ~~all~~ secrets ~~in the project~~ and list all secret versions. ~~Furthermore - this will require to effectively give SA access to all secrets in the project because otherwise it will raise an exception while trying to access secret version.~~
TLDR: It won't work if SA is given an access only permission ~~to a subset of project secrets~~ (on a secret level).

Update:
secretmanager.viewer can be given on a secret level as well so the previous statement is only a partially true

radcortez · 2023-09-06T23:36:33Z

Thanks!

It's a bit hard to test this. I couldn't find a proper way to emulate the SM, so I created my own and did some tests. Unfortunately, I'm not aware of all the options available.

We removed the ability that allowed us to use CDI during source creation because it caused many issues. It created chicken and egg scenarios, where you need config to start something, but you require something to create the config.

The previous implementation relied on CDI to provide the client, which CDI managed and closed. The Config components do not have a way to clean up resources, meaning that we either have to do it in the factories (as proposed), or we would need to create and close the client on each value lookup. I guess from what you describe there is no other option.

I'll have look again.

jaworskib · 2023-09-07T06:53:14Z

I had to update my previous comment as it was only a partially true.

Maybe the proposed approach isn't that bad at all but it definitely should be a well documented that this extension will try to access all secrets (and all it's versions) that can be listed by the credentials used and additional permissions are required (in the ideal world secretmanager.secretAccessor should be enough). However, it may lead to problems if someone will give SA secretmanager.viewer role on a project level and secretmanager.secretAccessor role only on a some subset of secrets on a secret level.

The other approach I can think of (beside the one with creating and closing the client on each value lookup) would be to scan all the properties with the sm// value format and load in factory only the used ones. Disclaimer: I'm not sure how feasible it is, I'm quite new in the quarkiverse.

loicmathieu · 2023-09-07T07:23:34Z

Hi, thanks @radcortez for the proposed fix and @jaworskib for the explanation.
I'll try to review it tomorrow and made some test on a real GCP project.

If I understand it correctly, the main difference is that now all secrets are retrieved at first call when previously they was retrieved one by one when needed.

The other approach I can think of (beside the one with creating and closing the client on each value lookup) would be to scan all the properties with the sm// value format and load in factory only the used ones. Disclaimer: I'm not sure how feasible it is, I'm quite new in the quarkiverse.

This seems a good idea as otherwise we will access secrets that are not needed, and API calls have cost in the cloud.

radcortez · 2023-09-07T09:53:34Z

The other approach I can think of (beside the one with creating and closing the client on each value lookup) would be to scan all the properties with the sm// value format and load in factory only the used ones. Disclaimer: I'm not sure how feasible it is, I'm quite new in the quarkiverse.

Unfortunately, I don't think this is feasible, because the sm// part is in the value and not in the property name, meaning that you have to evaluate every configuration for its value to find such patterns. While technically possible, I think it may cause a major performance hit.

Also, some configuration sources, do not expose all of their value, in the sense that if you query for a name you may get a value, but if you query for the list of names you get an empty list. In that case, we may be missing values that contain the sm// prefix.

Anyway, I have another idea to be able to have the client and clean it properly. I'll send a new update soon.

radcortez · 2023-09-07T11:58:54Z

Ok, I reverted to the old source, still without CDI, and added a way to close the client on shutdown. It should work for most cases.

The only problem is if an application is playing with Config instances (which they shouldn't), then these are required to do the cleanup themselves.

radcortez mentioned this issue Sep 6, 2023

Do not use CDI beans to read the SM #493

Merged

loicmathieu mentioned this issue Sep 11, 2023

Do not use CDI beans to read the SM #499

Merged

loicmathieu closed this as completed in #499 Sep 11, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

No longer possible to use secret-manager injected secret in @ConfigMapping class #491

No longer possible to use secret-manager injected secret in @ConfigMapping class #491

jaworskib commented Sep 2, 2023 •

edited

Loading

loicmathieu commented Sep 5, 2023

radcortez commented Sep 5, 2023

jaworskib commented Sep 5, 2023

radcortez commented Sep 6, 2023

radcortez commented Sep 6, 2023

jaworskib commented Sep 6, 2023 •

edited

Loading

radcortez commented Sep 6, 2023

jaworskib commented Sep 7, 2023 •

edited

Loading

loicmathieu commented Sep 7, 2023

radcortez commented Sep 7, 2023

radcortez commented Sep 7, 2023

No longer possible to use secret-manager injected secret in @ConfigMapping class #491

No longer possible to use secret-manager injected secret in @ConfigMapping class #491

Comments

jaworskib commented Sep 2, 2023 • edited Loading

loicmathieu commented Sep 5, 2023

radcortez commented Sep 5, 2023

jaworskib commented Sep 5, 2023

radcortez commented Sep 6, 2023

radcortez commented Sep 6, 2023

jaworskib commented Sep 6, 2023 • edited Loading

radcortez commented Sep 6, 2023

jaworskib commented Sep 7, 2023 • edited Loading

loicmathieu commented Sep 7, 2023

radcortez commented Sep 7, 2023

radcortez commented Sep 7, 2023

jaworskib commented Sep 2, 2023 •

edited

Loading

jaworskib commented Sep 6, 2023 •

edited

Loading

jaworskib commented Sep 7, 2023 •

edited

Loading