Paths differ in how we treat them in configmaps and secrets

[wind57]

Consider the case when you have enabled:

spring:
  cloud:
    kubernetes:
       config:
          paths:
            - /tmp/application.properties

and:

spring:
  cloud:
    kubernetes:
       secrets:
          paths:
            - /tmp

The first one expects an exact path for a config file, the second can actually "walk", see here.

While you can specify:

spring:
  cloud:
    kubernetes:
       secrets:
          paths:
            - /tmp/application.properties

walking is still an option, unlike in config maps.

In its fairness, this is documented correctly. First:

spring.cloud.kubernetes.config.paths expects a List of full paths to each property file, because directories are not being recursively parsed

and:

When enabled, the Fabric8SecretsPropertySource looks up Kubernetes for Secrets from the following sources:

Reading recursively from secrets mounts

Still, it feels a bit weird that Secrets are able to walk a directory and ConfigMaps can't.

Should this be unified and should we make ConfigMaps directories "walkable" too?

Or may be in reverse, make Secrets "unwalkable"? The problem in providing support for Files::walk is order. This method does not define any order of reading files, so if you have mounted application.yaml and application-prod.yaml, there might be potential issues from run to run.

[ryanjbaxter]

Interesting.

Or may be in reverse, make Secrets "unwalkable"?

I don't think so because that would be a breaking change, and we can't do that now (if we wanted to).

he problem in providing support for Files::walk is order. This method does not define any order of reading files, so if you have mounted application.yaml and application-prod.yaml, there might be potential issues from run to run.

I don't see profile specific files being an issue with order since anything with an active profile should be higher priority regardless of where Files.walk places them.
However it its just any random file, order changing from one application start to the next is an issue. In any case if we did it for configmaps we would at least match functionality. Maybe just document the unpredictability of the order of the resulting property sources?

I think we could also consider deprecating this support entirely because Spring Boot now offers this with spring.config.import.

[wind57]

thank you for the feedback, I'll fix this one, but I need to write proper integration tests. Some of them are already present here, so if you could take a look at that one, it would be much appreciated from me. thank you.

[ryanjbaxter]

I am actually leaning to not aligning them and deprecating this feature in the 2022 release and remove it in the next major release. Its duplicating what can be done with spring.config.import and the refresh functionality of mounted volumes can be achieved with the config watcher

[wind57]

especially since it's buggy, to say the least. I found a few more bugs there:

• ConfigMaps only support properties/yaml/yml files, they do not support plain properties
• Secrets have it in reverse, they only support plain properties, they do not support properties/yaml/yml files.
• We document this not in a very good way.

So to get your input correctly:

• no active development on paths support, only potential fixes for future bugs; much like we treat 2.1.x branch right now.
• replace it in the next major release with spring.config.import support.

Please correct me if I am wrong with anything.

P.S. I would be more then glad to look closer at config import support.

[ryanjbaxter]

Yup.

I think we just need to add a note to the documentation that the feature is deprecated and is scheduled for removal in the next major release.

We shouldn't need to do anything to support spring.config.import. You just need to mount the ConfigMap/Secret to the container and then set spring.config.import at the directory/file(s)/configtree for the mounted files.

[wind57]

We shouldn't need to do anything to support spring.config.import. You just need to mount the ConfigMap/Secret to the container and then set spring.config.import at the directory/file(s)/configtree for the mounted files.

Agreed. I just want to add/change integration tests. I'll look into this one and name it as NEXT MAJOR RELEASE

[wind57]

it seems that this issue is a duplicate of this one, now that we agreed to re-purpose this one.

We should close one as a duplicate of the other, no?

[ryanjbaxter]

Yup