Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Restore support for spring.profiles.include for non profile specific documents #22944

Closed
philwebb opened this issue Aug 13, 2020 · 9 comments
Closed
Assignees
Labels
theme: config-data Issues related to the configuration theme type: enhancement A general enhancement
Milestone

Comments

@philwebb
Copy link
Member

No description provided.

@philwebb philwebb self-assigned this Aug 13, 2020
@philwebb philwebb added the for: team-attention An issue we'd like other members of the team to review label Aug 13, 2020
@kedar-joshi
Copy link
Contributor

kedar-joshi commented Aug 14, 2020

Oh yes, please. spring.profiles.include is (was 😢) an important aspect of how we organize common configuration properties for all of our products. As the description goes, Unconditionally activate the specified comma-separated list of profiles is an important feature to have in order to avoid repeating common profile names everywhere.

Personally, I would prefer spring.profiles.include any time over spring.profiles.group.

(For experience sake) I migrated one of our production apps to Spring Boot v2.4.0-M2 and now suddenly I need to maintain configuration groups for all of our environments in application.properties which we used to manage with environmental variables.

Using multiple discrete profiles might make your code easier to reason about, but it’s not ideal for deployment. You don’t want to force users to remember that they must always activate proddb, prodmq, prodmetrics etc. at the same time. Instead, you just want them to be able to activate a single prod profile. Groups allow you to do just that.

I (somewhat) get this rationale but I (humbly) propose to make it an on-demand feature instead of enabling it by default (spring.profiles.include being the default).

P.S.
I know I have an option to go back to the old behavior and this is based on my own observations so I may be alone to think this way (but since this is brought to teams attention, I hope I am not alone).

@philwebb
Copy link
Member Author

@kedar-joshi Do you have any more details that you can share about how you currently use spring.profiles.include, perhaps a few sample config files that you can share?

I've very much trying to keep the new code simpler and prevent some of the issues that we had with the earlier version. Although I was considering if we should add back some form of spring.profiles.include support, I'm not keen to allow it to be combined with spring.config.activate.on-profile. It allows all sorts of impossible setups., e.g.:

spring.config.activate.on-profile: !a
spring.profiles.include: b
prop1: value1
---
spring.config.activate.on-profile: b
spring.profiles.include: a;
prop2: value2

@kedar-joshi
Copy link
Contributor

kedar-joshi commented Aug 15, 2020

Do you have any more details that you can share about how you currently use spring.profiles.include, perhaps a few sample config files that you can share ?

Sure.

We use spring.profiles.include primarily in two different ways -

1. Include + Active Profiles

application.properties

# Profile properties
spring.profiles.include = common
spring.profiles.active = ${PRODUCT_A_PROFILES:dev}

# e.g.
# PRODUCT_A_PROFILES = production,production-node-a

# Common application properties
# ...

application-production.properties

# Production environment specific properties
# ...

application-production-node-a.properties

# Node-A specific properties in production environment
# ...

2. (Only) Include Profiles

application.properties

# Profile properties
spring.profiles.include = ${PRODUCT_A_PROFILES:dev}

# e.g.
# PRODUCT_A_PROFILES = production-node-a

# Common application properties
# ...

application-flavor-a.properties

# Profile properties
spring.profiles.include = common

# Product flavor-a specific properties
# ...

application-production.properties

# Profile properties
spring.profiles.include = flavor-a

# Production environment specific properties
# ...

application-production-node-a.properties

# Profile properties
spring.profiles.include = production

# Node-A specific properties in production environment
# ...

Value for PRODUCT_A_PROFILES is provided by target environment.

Each method has its own denefits and challenges. Using spring.profiles.include and spring.profiles.active together considerably simplifies the configuration but it leaves a lot of room for incorrect configuration, during deployment, if there are too many profiles to be activated.

On the other hand, Using spring.profiles.include alone, simplifies deployment configuration at the cost of added complexity for developers.

We choose the method which best suits the product.

@philwebb
Copy link
Member Author

Thanks very much for the extra information.

I think option 1) wouldn't be too difficult for us to support since there are no spring.profiles.include properties in the profile specific files.

Option 2) is more tricky since we'd be back to not knowing which profiles are active before we start to process the profile specific files. We can make option 2 look like option 1 if we do something like this:

application.properties

# Profile properties
spring.profiles.include = ${PRODUCT_A_PROFILES:dev}
spring.profiles.include=${production-node-a:dev}
spring.profiles.group.production-node-a=production
spring.profiles.group.production=flavor-a
spring.profiles.group.flavor-a=common

# Common application properties
# ...

application-flavor-a.properties

# Product flavor-a specific properties
# ...

application-production.properties

# Production environment specific properties
# ...

application-production-node-a.properties

# Node-A specific properties in production environment
# ...

That's a obviously a bit more migration work, but it does allow us to determine exactly which profiles are active before we try to process profiles specific files/documents.

@philwebb philwebb added the status: waiting-for-triage An issue we've not yet triaged label Aug 17, 2020
@philwebb
Copy link
Member Author

See also the discussion in #21697

@kedar-joshi
Copy link
Contributor

kedar-joshi commented Aug 17, 2020

See also the discussion in #21697

Luckily we haven't faced any such issue because of the way we define our configuration.

That's a obviously a bit more migration work, but it does allow us to determine exactly which profiles are active before we try to process profiles specific files/documents.

I understand how this is required from Spring Boot's perspective so I played around a bit. The results / observations were quite interesting. Group name being added before all other profiles was the biggest surprise. E.g.

spring.profiles.group.production-node-a = production
spring.profiles.group.production = flavor-a
spring.profiles.group.flavor-a = common

With above setup, spring.profiles.active = production-node-a results in active profiles being set as production-node-a,production,flavor-a,common. So basically common profile overrides everything (which should be other way around).

This requires creation of pseudo group name (which doesn't have a profile file, just to make sure nothing gets accidentally overridden).

Anyway, the biggest hurdle for us in adopting spring.profiles.group was the need to (know and) hardcode all profile combinations in application.properties. I experimented a bit and realized that spring.profiles.group can be set via environmental variable. So now even though I can't use Option 2, I can always do this -

application.properties

spring.profiles.active = common

.. and set spring.profiles.group.common=production,production-node-a in startup script.

@kedar-joshi
Copy link
Contributor

kedar-joshi commented Aug 17, 2020

Also, I may have misinterpreted the scope / expectations of this issue. Your recent comment suggests that this issue is more about supporting spring.profiles.include with spring.profiles.group while I am more interested in having spring.profiles.include behavior available by default (with or without spring.profiles.group).

However, I believe the argument that spring.profiles.include is an important feature still stands considering spring.config.use-legacy-processing option may not survive indefinitely.

@jeffbswope
Copy link

Maybe this is too specific to Spring Cloud Config, but there we leverage spring.profiles.include in our {application-name}.properties to designate the shared resources that an application uses, like for instance spring.profiles.include=imrab,vxdatabase. The infrastructure launches the app solely with an "environment" profile -- so then on the config server side we have directories with files like {environment}/application-imrab.properties which coordinate and enable that subsystem for the application. This provides that:

  • the infrastructure can launch applications agnostic of which app it is and what subsystems it uses, simply providing the "environment profile"
  • the applications/jars themselves can remain ignorant of the deployment/configuration infrastructure profiles: the apps know and expose that they need (or optionally support) a Rabbit instance and/or a database and/or whatever else... but which instances exactly those are, or how they relate to the infrastructure's universe of AMQP/DB instances is not the applications' direct concern. We actually have too many infrastructures at this point so the tagged/versioned/validated release of an application shouldn't bake in affinity for any one in particular...
    • (This is an aside, but the fact that devs are guided or at least tempted to put dev-integration settings in the jar's application.properties to support bootRun/tests and therefore infrastructure bobbles can lead to those settings being silent defaults in a deployed application has led to headaches/workarounds to avoid that scenario...)
  • those coordinates/secrets are in one place and not repeated: e.g. all apps in the dev environment connecting to imrab source those settings from dev/application-imrab.properties

Perhaps (likely) we are making a mess of this and I'd be happy to hear advice. And as mentioned this might be more a Spring Cloud concern, as really what we logically want to do here is just literally "include" common/shared properties but we end up having to use (abuse) the profile system to get there?

I believe Spring Cloud Config does special processing on their end to "gather" included profiles from sources and return those dependent sources in the single request for property sources... so presumably they are affected by these changes in 2.4 although I haven't seen how they intend to adjust. Seems to be a similar problem to what is generally being solved here with discovering/ranking included profiles where that logic can theoretically chain infinitely and/or circularly.

@philwebb philwebb added type: enhancement A general enhancement and removed for: team-attention An issue we'd like other members of the team to review status: waiting-for-triage An issue we've not yet triaged labels Aug 19, 2020
@philwebb philwebb added this to the 2.2.x milestone Aug 19, 2020
@philwebb philwebb added the theme: config-data Issues related to the configuration theme label Aug 19, 2020
@philwebb philwebb modified the milestones: 2.2.x, 2.4.x Aug 19, 2020
@philwebb philwebb added type: task A general task and removed type: enhancement A general enhancement labels Aug 19, 2020
@philwebb philwebb changed the title Consider if we should restore support for spring.profiles.include Restore support for spring.profiles.include for non profile specific documents Aug 19, 2020
@philwebb philwebb added type: enhancement A general enhancement and removed type: task A general task labels Aug 19, 2020
@philwebb philwebb modified the milestones: 2.4.x, 2.4.0-M3 Aug 26, 2020
@philwebb
Copy link
Member Author

I've restored spring.profiles.include support, however, it's now limited such that it can only be used on non profile-specific documents.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
theme: config-data Issues related to the configuration theme type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

3 participants