Skip to content

Config Imports Should be default pass through the current profile(s) #5707

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

Closed
dhodun opened this issue Apr 19, 2021 · 2 comments · Fixed by #5846
Closed

Config Imports Should be default pass through the current profile(s) #5707

dhodun opened this issue Apr 19, 2021 · 2 comments · Fixed by #5846

Comments

@dhodun
Copy link

dhodun commented Apr 19, 2021

By default Importing a config (I think) is just running the default profile unless otherwise specified. We have multiple nestings of skaffold.yaml configs (for microservices and groups of microservices) and were expecting that profiles were inherited on the way down.

We would much prefer and expect that skaffold applies the current profile(s) unless otherwise specified.

Our root skaffold.yaml has gotten pretty messy, and is 50% profile activation code. We ultimately fixed with yaml anchors. But when we add a profile, we basically have to track down every single 'requires' section in all our skaffold yamls (not just root) to make sure we add the appropriate profiles.

This would be breaking change for anyone importing a config and just using the default profile (not many use cases for this I'm thinking?)

# Master Skaffold
apiVersion: skaffold/v2beta12
kind: Config
metadata:
  name: backends
requires:
- path: ./microservices/authentication
  configs: [authentication]
  activeProfiles:
  - name: custom
    activatedBy: [custom]
  - name: dev
    activatedBy: [dev]

- path: ./common
  configs: [common]
  activeProfiles: 
  - name: custom
    activatedBy: [custom] 
  - name: dev
    activatedBy: [dev] 

- path: ./microservices/dashboard
  configs: [dashboard]
  activeProfiles:
  - name: custom
    activatedBy: [custom]
  - name: dev
    activatedBy: [dev]

- path: ./microservices/messages
  configs: [messages]
  activeProfiles:
  - name: custom
    activatedBy: [custom]
  - name: dev
    activatedBy: [dev]
@briandealwis
Copy link
Member

briandealwis commented Apr 19, 2021

I do the same in container-debug-support. Perhaps we could provide for a block-list to forbid propagating profiles.

@gsquared94
Copy link
Contributor

gsquared94 commented Apr 20, 2021

I can agree with inverting the behavior: making all profiles apply to all configs recursively, and providing a mechanism to avoid selected profiles.

We could have a flag --recursively-applied-profiles with values like all (default), none, only-for=p1,p2,p3, not-for=p1,p2,p3

WDYT @briandealwis

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment