Skip to content

propagate profiles across imported configs by default; disable using propagate-profiles flag #5846

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.

Sign up for GitHub

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

Merged
merged 3 commits into from
May 14, 2021
Merged

propagate profiles across imported configs by default; disable using propagate-profiles flag #5846

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
1b8aebf
add `propagate-profiles` flag
gsquared94 May 14, 2021
bc0be25
update flag text
gsquared94 May 14, 2021
005868c
fix typo
gsquared94 May 14, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions cmd/skaffold/app/cmd/flags.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -367,6 +367,15 @@ var flagRegistry = []Flag{
DefinedOn: []string{"dev", "run", "debug", "deploy", "render", "build", "delete", "diagnose"},
IsEnum: true,
},
{
Name: "propagate-profiles",
Usage: "Setting '--propagate-profiles=false' disables propagating profiles set by the '--profile' flag across config dependencies. This mean that only profiles defined directly in the target 'skaffold.yaml' file are activated.",
Value: &opts.PropagateProfiles,
DefValue: true,
FlagAddMethod: "BoolVar",
DefinedOn: []string{"dev", "run", "debug", "deploy", "render", "build", "delete", "diagnose"},
IsEnum: true,
},
{
Name: "trigger",
Usage: "How is change detection triggered? (polling, notify, or manual)",
Expand Down
6 changes: 5 additions & 1 deletion docs/content/en/docs/design/config.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,9 @@ The remote config gets treated like a local config after substituting the path w

### Profile Activation in required configs

Additionally the `activeProfiles` stanza can define the profiles to be activated in the required configs, via:
Profiles specified by the `--profile` flag are also propagated to all configurations imported as dependencies, if they define them. This behavior can be disabled by setting the `--propagate-profiles` flag to `false`.

You can additionally set up more granular and conditional profile activations across dependencies through the `activeProfiles` stanza:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: it might helpful to add an example of how --propogate-profiles can be used w/ skaffold /examples directory if a user wants to see it in action easily

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

will take up as a followup item.


```yaml
apiVersion: skaffold/v2beta11
Expand All @@ -173,6 +175,8 @@ requires:

Here, `profile1` is a profile that needs to exist in both configs `cfg1` and `cfg2`; while `profile2` and `profile3` are profiles defined in the current config `cfg`. If the current config is activated with either `profile2` or `profile3` then the required configs `cfg1` and `cfg2` are imported with `profile1` applied. If the `activatedBy` clause is omitted then that `profile1` always gets applied for the imported configs.



{{< alert title="Follow up" >}}
Take a look at the [tutorial]({{< relref "/docs/tutorials/config-dependencies" >}}) section to see this in action.
{{< /alert >}}
16 changes: 16 additions & 0 deletions docs/content/en/docs/references/cli/_index.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -204,6 +204,7 @@ Options:
-o, --output={{json .}}: Used in conjunction with --quiet flag. Format output with go-template. For full struct documentation, see https://godoc.org/github.com/GoogleContainerTools/skaffold/cmd/skaffold/app/flags#BuildOutput
-p, --profile=[]: Activate profiles by name (prefixed with `-` to disable a profile)
--profile-auto-activation=true: Set to false to disable profile auto activation
--propagate-profiles=true: Setting '--propagate-profiles=false' disables propagating profiles set by the '--profile' flag across config dependencies. This mean that only profiles defined directly in the target 'skaffold.yaml' file are activated.
--push=: Push the built images to the specified image repository.
-q, --quiet=false: Suppress the build output and print image built on success. See --output to format output.
--remote-cache-dir='': Specify the location of the git repositories cache (default $HOME/.skaffold/repos)
Expand Down Expand Up @@ -242,6 +243,7 @@ Env vars:
* `SKAFFOLD_OUTPUT` (same as `--output`)
* `SKAFFOLD_PROFILE` (same as `--profile`)
* `SKAFFOLD_PROFILE_AUTO_ACTIVATION` (same as `--profile-auto-activation`)
* `SKAFFOLD_PROPAGATE_PROFILES` (same as `--propagate-profiles`)
* `SKAFFOLD_PUSH` (same as `--push`)
* `SKAFFOLD_QUIET` (same as `--quiet`)
* `SKAFFOLD_REMOTE_CACHE_DIR` (same as `--remote-cache-dir`)
Expand Down Expand Up @@ -432,6 +434,7 @@ Options:
--port-forward=user,debug: Port-forward exposes service ports and container ports within pods and other resources (off, user, services, debug, pods)
-p, --profile=[]: Activate profiles by name (prefixed with `-` to disable a profile)
--profile-auto-activation=true: Set to false to disable profile auto activation
--propagate-profiles=true: Setting '--propagate-profiles=false' disables propagating profiles set by the '--profile' flag across config dependencies. This mean that only profiles defined directly in the target 'skaffold.yaml' file are activated.
--remote-cache-dir='': Specify the location of the git repositories cache (default $HOME/.skaffold/repos)
--rpc-http-port=50052: tcp port to expose event REST API over HTTP
--rpc-port=50051: tcp port to expose event API
Expand Down Expand Up @@ -484,6 +487,7 @@ Env vars:
* `SKAFFOLD_PORT_FORWARD` (same as `--port-forward`)
* `SKAFFOLD_PROFILE` (same as `--profile`)
* `SKAFFOLD_PROFILE_AUTO_ACTIVATION` (same as `--profile-auto-activation`)
* `SKAFFOLD_PROPAGATE_PROFILES` (same as `--propagate-profiles`)
* `SKAFFOLD_REMOTE_CACHE_DIR` (same as `--remote-cache-dir`)
* `SKAFFOLD_RPC_HTTP_PORT` (same as `--rpc-http-port`)
* `SKAFFOLD_RPC_PORT` (same as `--rpc-port`)
Expand Down Expand Up @@ -518,6 +522,7 @@ Options:
-n, --namespace='': Run deployments in the specified namespace
-p, --profile=[]: Activate profiles by name (prefixed with `-` to disable a profile)
--profile-auto-activation=true: Set to false to disable profile auto activation
--propagate-profiles=true: Setting '--propagate-profiles=false' disables propagating profiles set by the '--profile' flag across config dependencies. This mean that only profiles defined directly in the target 'skaffold.yaml' file are activated.
--remote-cache-dir='': Specify the location of the git repositories cache (default $HOME/.skaffold/repos)

Usage:
Expand All @@ -539,6 +544,7 @@ Env vars:
* `SKAFFOLD_NAMESPACE` (same as `--namespace`)
* `SKAFFOLD_PROFILE` (same as `--profile`)
* `SKAFFOLD_PROFILE_AUTO_ACTIVATION` (same as `--profile-auto-activation`)
* `SKAFFOLD_PROPAGATE_PROFILES` (same as `--propagate-profiles`)
* `SKAFFOLD_REMOTE_CACHE_DIR` (same as `--remote-cache-dir`)

### skaffold deploy
Expand Down Expand Up @@ -580,6 +586,7 @@ Options:
--port-forward=off: Port-forward exposes service ports and container ports within pods and other resources (off, user, services, debug, pods)
-p, --profile=[]: Activate profiles by name (prefixed with `-` to disable a profile)
--profile-auto-activation=true: Set to false to disable profile auto activation
--propagate-profiles=true: Setting '--propagate-profiles=false' disables propagating profiles set by the '--profile' flag across config dependencies. This mean that only profiles defined directly in the target 'skaffold.yaml' file are activated.
--remote-cache-dir='': Specify the location of the git repositories cache (default $HOME/.skaffold/repos)
--rpc-http-port=50052: tcp port to expose event REST API over HTTP
--rpc-port=50051: tcp port to expose event API
Expand Down Expand Up @@ -620,6 +627,7 @@ Env vars:
* `SKAFFOLD_PORT_FORWARD` (same as `--port-forward`)
* `SKAFFOLD_PROFILE` (same as `--profile`)
* `SKAFFOLD_PROFILE_AUTO_ACTIVATION` (same as `--profile-auto-activation`)
* `SKAFFOLD_PROPAGATE_PROFILES` (same as `--propagate-profiles`)
* `SKAFFOLD_REMOTE_CACHE_DIR` (same as `--remote-cache-dir`)
* `SKAFFOLD_RPC_HTTP_PORT` (same as `--rpc-http-port`)
* `SKAFFOLD_RPC_PORT` (same as `--rpc-port`)
Expand Down Expand Up @@ -669,6 +677,7 @@ Options:
--port-forward=user: Port-forward exposes service ports and container ports within pods and other resources (off, user, services, debug, pods)
-p, --profile=[]: Activate profiles by name (prefixed with `-` to disable a profile)
--profile-auto-activation=true: Set to false to disable profile auto activation
--propagate-profiles=true: Setting '--propagate-profiles=false' disables propagating profiles set by the '--profile' flag across config dependencies. This mean that only profiles defined directly in the target 'skaffold.yaml' file are activated.
--remote-cache-dir='': Specify the location of the git repositories cache (default $HOME/.skaffold/repos)
--rpc-http-port=50052: tcp port to expose event REST API over HTTP
--rpc-port=50051: tcp port to expose event API
Expand Down Expand Up @@ -722,6 +731,7 @@ Env vars:
* `SKAFFOLD_PORT_FORWARD` (same as `--port-forward`)
* `SKAFFOLD_PROFILE` (same as `--profile`)
* `SKAFFOLD_PROFILE_AUTO_ACTIVATION` (same as `--profile-auto-activation`)
* `SKAFFOLD_PROPAGATE_PROFILES` (same as `--propagate-profiles`)
* `SKAFFOLD_REMOTE_CACHE_DIR` (same as `--remote-cache-dir`)
* `SKAFFOLD_RPC_HTTP_PORT` (same as `--rpc-http-port`)
* `SKAFFOLD_RPC_PORT` (same as `--rpc-port`)
Expand Down Expand Up @@ -758,6 +768,7 @@ Options:
-m, --module=[]: Filter Skaffold configs to only the provided named modules
-p, --profile=[]: Activate profiles by name (prefixed with `-` to disable a profile)
--profile-auto-activation=true: Set to false to disable profile auto activation
--propagate-profiles=true: Setting '--propagate-profiles=false' disables propagating profiles set by the '--profile' flag across config dependencies. This mean that only profiles defined directly in the target 'skaffold.yaml' file are activated.
--remote-cache-dir='': Specify the location of the git repositories cache (default $HOME/.skaffold/repos)
--yaml-only=false: Only prints the effective skaffold.yaml configuration

Expand All @@ -775,6 +786,7 @@ Env vars:
* `SKAFFOLD_MODULE` (same as `--module`)
* `SKAFFOLD_PROFILE` (same as `--profile`)
* `SKAFFOLD_PROFILE_AUTO_ACTIVATION` (same as `--profile-auto-activation`)
* `SKAFFOLD_PROPAGATE_PROFILES` (same as `--propagate-profiles`)
* `SKAFFOLD_REMOTE_CACHE_DIR` (same as `--remote-cache-dir`)
* `SKAFFOLD_YAML_ONLY` (same as `--yaml-only`)

Expand Down Expand Up @@ -898,6 +910,7 @@ Options:
-o, --output='': file to write rendered manifests to
-p, --profile=[]: Activate profiles by name (prefixed with `-` to disable a profile)
--profile-auto-activation=true: Set to false to disable profile auto activation
--propagate-profiles=true: Setting '--propagate-profiles=false' disables propagating profiles set by the '--profile' flag across config dependencies. This mean that only profiles defined directly in the target 'skaffold.yaml' file are activated.
--remote-cache-dir='': Specify the location of the git repositories cache (default $HOME/.skaffold/repos)

Usage:
Expand All @@ -923,6 +936,7 @@ Env vars:
* `SKAFFOLD_OUTPUT` (same as `--output`)
* `SKAFFOLD_PROFILE` (same as `--profile`)
* `SKAFFOLD_PROFILE_AUTO_ACTIVATION` (same as `--profile-auto-activation`)
* `SKAFFOLD_PROPAGATE_PROFILES` (same as `--propagate-profiles`)
* `SKAFFOLD_REMOTE_CACHE_DIR` (same as `--remote-cache-dir`)

### skaffold run
Expand Down Expand Up @@ -966,6 +980,7 @@ Options:
--port-forward=off: Port-forward exposes service ports and container ports within pods and other resources (off, user, services, debug, pods)
-p, --profile=[]: Activate profiles by name (prefixed with `-` to disable a profile)
--profile-auto-activation=true: Set to false to disable profile auto activation
--propagate-profiles=true: Setting '--propagate-profiles=false' disables propagating profiles set by the '--profile' flag across config dependencies. This mean that only profiles defined directly in the target 'skaffold.yaml' file are activated.
--remote-cache-dir='': Specify the location of the git repositories cache (default $HOME/.skaffold/repos)
--rpc-http-port=50052: tcp port to expose event REST API over HTTP
--rpc-port=50051: tcp port to expose event API
Expand Down Expand Up @@ -1014,6 +1029,7 @@ Env vars:
* `SKAFFOLD_PORT_FORWARD` (same as `--port-forward`)
* `SKAFFOLD_PROFILE` (same as `--profile`)
* `SKAFFOLD_PROFILE_AUTO_ACTIVATION` (same as `--profile-auto-activation`)
* `SKAFFOLD_PROPAGATE_PROFILES` (same as `--propagate-profiles`)
* `SKAFFOLD_REMOTE_CACHE_DIR` (same as `--remote-cache-dir`)
* `SKAFFOLD_RPC_HTTP_PORT` (same as `--rpc-http-port`)
* `SKAFFOLD_RPC_PORT` (same as `--rpc-port`)
Expand Down
1 change: 1 addition & 0 deletions pkg/skaffold/config/options.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ type SkaffoldOptions struct {
DryRun bool
SkipRender bool
SkipConfigDefaults bool
PropagateProfiles bool

// Add Skaffold-specific labels including runID, deployer labels, etc.
// `CustomLabels` are still applied if this is false. Must only be used in
Expand Down
11 changes: 10 additions & 1 deletion pkg/skaffold/parser/config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,16 @@ func processEachConfig(config *latestV1.SkaffoldConfig, cfgOpts configOpts, opts

var configs SkaffoldConfigSet
for _, d := range config.Dependencies {
newOpts := configOpts{file: cfgOpts.file, profiles: filterActiveProfiles(d, profiles), isRequired: required, isDependency: cfgOpts.isDependency}
depProfiles := filterActiveProfiles(d, profiles)
if opts.PropagateProfiles {
// propagate all profiles supplied as command line input to the imported configs
for _, p := range opts.Profiles {
if !util.StrSliceContains(depProfiles, p) {
depProfiles = append(depProfiles, p)
}
}
}
newOpts := configOpts{file: cfgOpts.file, profiles: depProfiles, isRequired: required, isDependency: cfgOpts.isDependency}
depConfigs, err := processEachDependency(d, newOpts, opts, r)
if err != nil {
return nil, err
Expand Down
96 changes: 89 additions & 7 deletions pkg/skaffold/parser/config_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,13 +86,14 @@ type mockCfg struct {

func TestGetAllConfigs(t *testing.T) {
tests := []struct {
description string
documents []document
configFilter []string
profiles []string
makePathsAbsolute *bool
errCode proto.StatusCode
expected func(base string) []*latestV1.SkaffoldConfig
description string
documents []document
configFilter []string
profiles []string
makePathsAbsolute *bool
errCode proto.StatusCode
applyProfilesRecursively bool
expected func(base string) []*latestV1.SkaffoldConfig
}{
{
description: "makePathsAbsolute unspecified; no dependencies",
Expand Down Expand Up @@ -1205,6 +1206,86 @@ requires:
}
},
},
{
description: "makePathsAbsolute unspecified; recursively applied profiles",
profiles: []string{"pf0"},
applyProfilesRecursively: true,
documents: []document{
{path: "skaffold.yaml", configs: []mockCfg{{name: "cfg00", requiresStanza: `
requires:
- path: doc1
configs: [cfg10]
`}, {name: "cfg01", requiresStanza: ""}}},
{path: "doc1/skaffold.yaml", configs: []mockCfg{{name: "cfg10", requiresStanza: `
requires:
- path: ../doc2
configs: [cfg21]
`}, {name: "cfg11", requiresStanza: ""}}},
{path: "doc2/skaffold.yaml", configs: []mockCfg{{name: "cfg20", requiresStanza: ""}, {name: "cfg21", requiresStanza: ""}}},
},
expected: func(base string) []*latestV1.SkaffoldConfig {
return []*latestV1.SkaffoldConfig{
createCfg("cfg21", "pf0image21", filepath.Join(base, "doc2"), nil),
createCfg("cfg10", "pf0image10", filepath.Join(base, "doc1"), []latestV1.ConfigDependency{{Path: filepath.Join(base, "doc2"), Names: []string{"cfg21"}}}),
createCfg("cfg00", "pf0image00", ".", []latestV1.ConfigDependency{{Path: "doc1", Names: []string{"cfg10"}}}),
createCfg("cfg01", "pf0image01", ".", nil),
}
},
},
{
description: "makePathsAbsolute false; recursively applied profiles",
makePathsAbsolute: util.BoolPtr(false),
profiles: []string{"pf0"},
applyProfilesRecursively: true,
documents: []document{
{path: "skaffold.yaml", configs: []mockCfg{{name: "cfg00", requiresStanza: `
requires:
- path: doc1
configs: [cfg10]
`}, {name: "cfg01", requiresStanza: ""}}},
{path: "doc1/skaffold.yaml", configs: []mockCfg{{name: "cfg10", requiresStanza: `
requires:
- path: ../doc2
configs: [cfg21]
`}, {name: "cfg11", requiresStanza: ""}}},
{path: "doc2/skaffold.yaml", configs: []mockCfg{{name: "cfg20", requiresStanza: ""}, {name: "cfg21", requiresStanza: ""}}},
},
expected: func(base string) []*latestV1.SkaffoldConfig {
return []*latestV1.SkaffoldConfig{
createCfg("cfg21", "pf0image21", ".", nil),
createCfg("cfg10", "pf0image10", ".", []latestV1.ConfigDependency{{Path: "../doc2", Names: []string{"cfg21"}}}),
createCfg("cfg00", "pf0image00", ".", []latestV1.ConfigDependency{{Path: "doc1", Names: []string{"cfg10"}}}),
createCfg("cfg01", "pf0image01", ".", nil),
}
},
},
{
description: "makePathsAbsolute true; recursively applied profiles",
makePathsAbsolute: util.BoolPtr(true),
profiles: []string{"pf0"},
applyProfilesRecursively: true,
documents: []document{
{path: "skaffold.yaml", configs: []mockCfg{{name: "cfg00", requiresStanza: `
requires:
- path: doc1
configs: [cfg10]
`}, {name: "cfg01", requiresStanza: ""}}},
{path: "doc1/skaffold.yaml", configs: []mockCfg{{name: "cfg10", requiresStanza: `
requires:
- path: ../doc2
configs: [cfg21]
`}, {name: "cfg11", requiresStanza: ""}}},
{path: "doc2/skaffold.yaml", configs: []mockCfg{{name: "cfg20", requiresStanza: ""}, {name: "cfg21", requiresStanza: ""}}},
},
expected: func(base string) []*latestV1.SkaffoldConfig {
return []*latestV1.SkaffoldConfig{
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: config_test.go is a bit long atm with 56 test cases in TestGetAllConfigs. From looking at it, this is likely justified as we want the full set of combos for each config option and the test framework is the same for all of them but wanted to point it out as it might be hard for newer users to read.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree but didn't find any obvious way to simplify it. Do you have any ideas how this test can be better structured?

createCfg("cfg21", "pf0image21", filepath.Join(base, "doc2"), nil),
createCfg("cfg10", "pf0image10", filepath.Join(base, "doc1"), []latestV1.ConfigDependency{{Path: filepath.Join(base, "doc2"), Names: []string{"cfg21"}}}),
createCfg("cfg00", "pf0image00", base, []latestV1.ConfigDependency{{Path: filepath.Join(base, "doc1"), Names: []string{"cfg10"}}}),
createCfg("cfg01", "pf0image01", base, nil),
}
},
},
}
for _, test := range tests {
testutil.Run(t, test.description, func(t *testutil.T) {
Expand All @@ -1230,6 +1311,7 @@ requires:
ConfigurationFile: test.documents[0].path,
ConfigurationFilter: test.configFilter,
Profiles: test.profiles,
PropagateProfiles: test.applyProfilesRecursively,
MakePathsAbsolute: test.makePathsAbsolute,
})
if test.errCode == proto.StatusCode_OK {
Expand Down