Spring Boot 3 - cannot load secrets when 'list' role not configured for 'secrets' resource

[cg1410]

Describe the bug
In the previous versions of that library (2.1.3) we were able to load secrets on kubernetes cluster without 'list' permission given in the role for Service Account.
For example:
bootstrap configuration

spring:
  config:
    activate:
      on-profile: kubernetes
  main:
    banner-mode: "off"
  cloud:
    kubernetes:
      config:
        enabled: true
      discovery:
        enabled: true
      reload:
        enabled: true
      secrets:
        enabled: true
        enable-api: true
        sources:
          - name: some-secret-name

and Role:

apiVersion: rbac.authorization.k8s.io/v1
kind: Role
metadata:
....
rules:
  - apiGroups: ["", "extensions", "apps"]
    resources: ["secrets"]
   ...
    verbs: ["get", "watch"]

Then, in the microservice logs there was a warn that application got 403 error when trying to list secrets in the namespace: "Message: Forbidden! ... doesn't have permission. secrets is forbidden: User "..." cannot list secrets in the namespace "

But still, library was able to load secrets, and in the logs we could see the information:
[BootstrapPropertySource {name='bootstrapProperties-secrets.some-secret-name.some-microservice-namespace'},

After upgrading to Spring Boot 3.0 wiith the spring-cloud-kubernetes in the version 3.0.3

<dependency>
    <groupId>org.springframework.cloud</groupId>
    <artifactId>spring-cloud-starter-kubernetes-fabric8-all</artifactId>
    <version>3.0.3</version>
</dependency>

that functionality no longer works.
And in the application logs it is clearly visible why:
[BootstrapPropertySource {name='bootstrapProperties-secrets..some-microservice-namespace'},

The secret name is missing from located property source.
After debugging I figured out why is that happening.
Here is the code of class org.springframework.cloud.kubernetes.commons.config.NamedSourceData

spring-cloud-kubernetes/spring-cloud-kubernetes-commons/src/main/java/org/springframework/cloud/kubernetes/commons/config/NamedSourceData.java

Lines 51 to 70 in 4b2e362

	data = dataSupplier(sourceNames);
	if (data.names().isEmpty()) {
	return new SourceData(ConfigUtils.sourceName(target, sourceName, namespace), Map.of());
	}
	if (prefix != ConfigUtils.Prefix.DEFAULT) {
	// since we are in a named source, calling get on the supplier is safe
	String prefixToUse = prefix.prefixProvider().get();
	PrefixContext prefixContext = new PrefixContext(data.data(), prefixToUse, namespace, data.names());
	return ConfigUtils.withPrefix(target, prefixContext);
	}
	}
	catch (Exception e) {
	onException(failFast, e);
	}
	String names = data.names().stream().sorted().collect(Collectors.joining(PROPERTY_SOURCE_NAME_SEPARATOR));
	return new SourceData(ConfigUtils.sourceName(target, names, namespace), data.data());

in the line 51 data = dataSupplier(sourceNames) the Kubernetes Client is throwing an exception and because of that, variable data is still empty - so it DOES NOT contain secret names. So then in the last line before returrn the variable names will be empty. And the result is following:
[BootstrapPropertySource {name='bootstrapProperties-secrets..some-microservice-namespace'},

The question is: is there any way to change that behavior? We would like to stick with current Role solution - we don't want to give container a privilege to list all secrets in the namespace.
As i can see it could probably even work if the dataSupplier would not throw an exception - then that if statement from line 53
could probably make that working.
The other possibility is to call that if statement just before the return statement.

Please let me know If that is intended behavior or maybe it could be fixed.

[witek1902]

We noticed exactly the same problem.
IMO, this is a breaking change in library that should either be customizable or should not occur.

[wind57]

A breaking change is allowed on major upgrades.

I am a bit stuck with other things at the moment, but once I am a bit more free I will comment here more. Thank you for raising this.

[wind57]

Indeed we changed the way we read secrets/configmaps starting in release version 3.

The reasoning for this is rather simple: instead of reading potentially many sources one at a time, we opted for reading them all at once and them filtering out what is needed or not. For that to happen, we need the list verb indeed (instead of the previous get).

Since this was a change introduced in the next major release (3), such changes are allowed. The bad news is that I don't think we documented this.

As to "fixing" this: to me, there is nothing to fix here, we made this change on purpose for the reasons stated above.
If we ever want to provide a way for users to either chose bulk reads (list) or individual reads (get), is a complicated decision. It is doable, but the amount of code to maintain for these two logical branches would be very big.

One more point here is that reading sources via the k8s api server, is discouraged by us (by other people too), you might want to look at mounting these as volumes and then use spring.config.import with configtree.

[wind57]

@ryanjbaxter I don't think there is much to do here, unless you want me to add some proper notes in the documentation. We already have a section for breaking changes under 3.0.x. wdyt?

[ryanjbaxter]

It couldn’t hurt to add notes to the breaking changes section

[wind57]

on it!

[emlagowski]

Hey, we also noticed a problem caused by this change.

I understand that the change of major version allows you to create a breaking change, but is it a desired change? In the previous version, it was possible to list specific secret names that the application can get. The current approach forces you to allow ServiceAccount to retrieve all the secrets within the namespace, read them and filter them in the application. Why give such permissions to the application?

It was more secure before that change. If two or more applications share one namespace (for whatever reason) it forces us to allow them to read all secerets.

Also reading all sources at once when there are a lot of sources and some of them could be large seems problematic.

I think we can help with fixing that, but we need to know if it gonna be merged :)

[wind57]

If that is indeed a problem/inconvenience, you have always the option to disable the api calls at all, and mount these as paths. Just keep in mind that paths support will be dropped in the next major release and will be replaced with spring.config.import; meaning it will be part of 3.0.x, but most probably not as part of 4.0.x, whenever that happens.

As to fixing it, contributors I assume are always welcome (I am one myself :) ). Since list support is already out, you will need to create such a PR that maintains both, obviously. I am also not a spring employee, so such decisions are not entirely on me. I can only provide my feedback towards them.