feat: allow and deny list support for schema field overwriting (namel…

…y label and image fields atm)
GoogleContainerTools · Mar 4, 2022 · ce8ef96 · ce8ef96
1 parent d2134aa
commit ce8ef96
Show file tree

Hide file tree

Showing 24 changed files with 856 additions and 206 deletions.
diff --git a/cmd/skaffold/app/cmd/flags.go b/cmd/skaffold/app/cmd/flags.go
@@ -606,6 +606,14 @@ var flagRegistry = []Flag{
 		FlagAddMethod: "BoolVar",
 		DefinedOn:     []string{"deploy"},
 	},
+	{
+		Name:          "transform-rules-file",
+		Usage:         "Path to JSON file specifying the deny list of yaml objects for skaffold to NOT transform with 'image' and 'label' field replacements.  NOTE: this list is additive to skaffold's default denylist and denylist has priority over allowlist",
+		Value:         &opts.TransformRulesFile,
+		DefValue:      "",
+		FlagAddMethod: "StringVar",
+		DefinedOn:     []string{"dev", "render", "run", "debug", "deploy"},
+	},
 }
 
 func methodNameByType(v reflect.Value) string {

diff --git a/docs/content/en/docs/references/cli/_index.md b/docs/content/en/docs/references/cli/_index.md
@@ -430,6 +430,8 @@ Options:
   -t, --tag='': The optional custom tag to use for images which overrides the current Tagger configuration
       --tail=true: Stream logs from deployed objects
       --toot=false: Emit a terminal beep after the deploy is complete
+      --transform-allow-file='': Path to JSON file specifying the allow list of yaml objects for skaffold to NOT transform with 'image' and 'label' field replacements. NOTE: this list is additive to skaffold's default allowlist and denylist has priority over allowlist
+      --transform-deny-file='': Path to JSON file specifying the deny list of yaml objects for skaffold to NOT transform with 'image' and 'label' field replacements.  NOTE: this list is additive to skaffold's default denylist and denylist has priority over allowlist
       --trigger='notify': How is change detection triggered? (polling, notify, or manual)
       --wait-for-connection=false: Blocks ending execution of skaffold until the /v2/events gRPC/HTTP endpoint is hit
       --wait-for-deletions=true: Wait for pending deletions to complete before a deployment
@@ -486,6 +488,8 @@ Env vars:
 * `SKAFFOLD_TAG` (same as `--tag`)
 * `SKAFFOLD_TAIL` (same as `--tail`)
 * `SKAFFOLD_TOOT` (same as `--toot`)
+* `SKAFFOLD_TRANSFORM_ALLOW_FILE` (same as `--transform-allow-file`)
+* `SKAFFOLD_TRANSFORM_DENY_FILE` (same as `--transform-deny-file`)
 * `SKAFFOLD_TRIGGER` (same as `--trigger`)
 * `SKAFFOLD_WAIT_FOR_CONNECTION` (same as `--wait-for-connection`)
 * `SKAFFOLD_WAIT_FOR_DELETIONS` (same as `--wait-for-deletions`)
@@ -596,6 +600,8 @@ Options:
   -t, --tag='': The optional custom tag to use for images which overrides the current Tagger configuration
       --tail=false: Stream logs from deployed objects
       --toot=false: Emit a terminal beep after the deploy is complete
+      --transform-allow-file='': Path to JSON file specifying the allow list of yaml objects for skaffold to NOT transform with 'image' and 'label' field replacements. NOTE: this list is additive to skaffold's default allowlist and denylist has priority over allowlist
+      --transform-deny-file='': Path to JSON file specifying the deny list of yaml objects for skaffold to NOT transform with 'image' and 'label' field replacements.  NOTE: this list is additive to skaffold's default denylist and denylist has priority over allowlist
       --wait-for-connection=false: Blocks ending execution of skaffold until the /v2/events gRPC/HTTP endpoint is hit
       --wait-for-deletions=true: Wait for pending deletions to complete before a deployment
       --wait-for-deletions-delay=2s: Delay between two checks for pending deletions
@@ -640,6 +646,8 @@ Env vars:
 * `SKAFFOLD_TAG` (same as `--tag`)
 * `SKAFFOLD_TAIL` (same as `--tail`)
 * `SKAFFOLD_TOOT` (same as `--toot`)
+* `SKAFFOLD_TRANSFORM_ALLOW_FILE` (same as `--transform-allow-file`)
+* `SKAFFOLD_TRANSFORM_DENY_FILE` (same as `--transform-deny-file`)
 * `SKAFFOLD_WAIT_FOR_CONNECTION` (same as `--wait-for-connection`)
 * `SKAFFOLD_WAIT_FOR_DELETIONS` (same as `--wait-for-deletions`)
 * `SKAFFOLD_WAIT_FOR_DELETIONS_DELAY` (same as `--wait-for-deletions-delay`)
@@ -692,6 +700,8 @@ Options:
   -t, --tag='': The optional custom tag to use for images which overrides the current Tagger configuration
       --tail=true: Stream logs from deployed objects
       --toot=false: Emit a terminal beep after the deploy is complete
+      --transform-allow-file='': Path to JSON file specifying the allow list of yaml objects for skaffold to NOT transform with 'image' and 'label' field replacements. NOTE: this list is additive to skaffold's default allowlist and denylist has priority over allowlist
+      --transform-deny-file='': Path to JSON file specifying the deny list of yaml objects for skaffold to NOT transform with 'image' and 'label' field replacements.  NOTE: this list is additive to skaffold's default denylist and denylist has priority over allowlist
       --trigger='notify': How is change detection triggered? (polling, notify, or manual)
       --wait-for-connection=false: Blocks ending execution of skaffold until the /v2/events gRPC/HTTP endpoint is hit
       --wait-for-deletions=true: Wait for pending deletions to complete before a deployment
@@ -748,6 +758,8 @@ Env vars:
 * `SKAFFOLD_TAG` (same as `--tag`)
 * `SKAFFOLD_TAIL` (same as `--tail`)
 * `SKAFFOLD_TOOT` (same as `--toot`)
+* `SKAFFOLD_TRANSFORM_ALLOW_FILE` (same as `--transform-allow-file`)
+* `SKAFFOLD_TRANSFORM_DENY_FILE` (same as `--transform-deny-file`)
 * `SKAFFOLD_TRIGGER` (same as `--trigger`)
 * `SKAFFOLD_WAIT_FOR_CONNECTION` (same as `--wait-for-connection`)
 * `SKAFFOLD_WAIT_FOR_DELETIONS` (same as `--wait-for-deletions`)
@@ -1017,6 +1029,8 @@ Options:
   -t, --tag='': The optional custom tag to use for images which overrides the current Tagger configuration
       --tail=false: Stream logs from deployed objects
       --toot=false: Emit a terminal beep after the deploy is complete
+      --transform-allow-file='': Path to JSON file specifying the allow list of yaml objects for skaffold to NOT transform with 'image' and 'label' field replacements. NOTE: this list is additive to skaffold's default allowlist and denylist has priority over allowlist
+      --transform-deny-file='': Path to JSON file specifying the deny list of yaml objects for skaffold to NOT transform with 'image' and 'label' field replacements.  NOTE: this list is additive to skaffold's default denylist and denylist has priority over allowlist
       --wait-for-connection=false: Blocks ending execution of skaffold until the /v2/events gRPC/HTTP endpoint is hit
       --wait-for-deletions=true: Wait for pending deletions to complete before a deployment
       --wait-for-deletions-delay=2s: Delay between two checks for pending deletions
@@ -1068,6 +1082,8 @@ Env vars:
 * `SKAFFOLD_TAG` (same as `--tag`)
 * `SKAFFOLD_TAIL` (same as `--tail`)
 * `SKAFFOLD_TOOT` (same as `--toot`)
+* `SKAFFOLD_TRANSFORM_ALLOW_FILE` (same as `--transform-allow-file`)
+* `SKAFFOLD_TRANSFORM_DENY_FILE` (same as `--transform-deny-file`)
 * `SKAFFOLD_WAIT_FOR_CONNECTION` (same as `--wait-for-connection`)
 * `SKAFFOLD_WAIT_FOR_DELETIONS` (same as `--wait-for-deletions`)
 * `SKAFFOLD_WAIT_FOR_DELETIONS_DELAY` (same as `--wait-for-deletions-delay`)

diff --git a/docs/content/en/schemas/v2beta28.json b/docs/content/en/schemas/v2beta28.json
@@ -3308,6 +3308,11 @@
           "type": "array",
           "description": "describes how images are tested.",
           "x-intellij-html-description": "describes how images are tested."
+        },
+        "transform": {
+          "$ref": "#/definitions/ResourceSelectorConfig",
+          "description": "describes user defined filters describing how skaffold should treat objects/fields during rendering.",
+          "x-intellij-html-description": "describes user defined filters describing how skaffold should treat objects/fields during rendering."
         }
       },
       "preferredOrder": [
@@ -3317,7 +3322,8 @@
         "build",
         "test",
         "deploy",
-        "portForward"
+        "portForward",
+        "transform"
       ],
       "additionalProperties": false,
       "type": "object",
@@ -3355,9 +3361,14 @@
     },
     "ResourceFilter": {
       "required": [
-        "type"
+        "groupKind"
       ],
       "properties": {
+        "groupKind": {
+          "type": "string",
+          "description": "compact format of a resource type.",
+          "x-intellij-html-description": "compact format of a resource type."
+        },
         "image": {
           "items": {
             "type": "string"
@@ -3375,15 +3386,10 @@
           "description": "an optional slide of JSON-path-like paths of where to add a labels block if missing.",
           "x-intellij-html-description": "an optional slide of JSON-path-like paths of where to add a labels block if missing.",
           "default": "[]"
-        },
-        "type": {
-          "type": "string",
-          "description": "compact format of a resource type.",
-          "x-intellij-html-description": "compact format of a resource type."
         }
       },
       "preferredOrder": [
-        "type",
+        "groupKind",
         "image",
         "labels"
       ],
@@ -3460,6 +3466,34 @@
       "description": "describes the resource requirements for the kaniko pod.",
       "x-intellij-html-description": "describes the resource requirements for the kaniko pod."
     },
+    "ResourceSelectorConfig": {
+      "properties": {
+        "allow": {
+          "items": {
+            "$ref": "#/definitions/ResourceFilter"
+          },
+          "type": "array",
+          "description": "configures an allowlist for transforming manifests.",
+          "x-intellij-html-description": "configures an allowlist for transforming manifests."
+        },
+        "deny": {
+          "items": {
+            "$ref": "#/definitions/ResourceFilter"
+          },
+          "type": "array",
+          "description": "configures an allowlist for transforming manifests.",
+          "x-intellij-html-description": "configures an allowlist for transforming manifests."
+        }
+      },
+      "preferredOrder": [
+        "allow",
+        "deny"
+      ],
+      "additionalProperties": false,
+      "type": "object",
+      "description": "contains all the configuration needed by the deploy steps.",
+      "x-intellij-html-description": "contains all the configuration needed by the deploy steps."
+    },
     "ResourceType": {
       "type": "string",
       "description": "describes the Kubernetes resource types used for port forwarding.",
@@ -3533,6 +3567,11 @@
           "type": "array",
           "description": "describes how images are tested.",
           "x-intellij-html-description": "describes how images are tested."
+        },
+        "transform": {
+          "$ref": "#/definitions/ResourceSelectorConfig",
+          "description": "describes user defined filters describing how skaffold should treat objects/fields during rendering.",
+          "x-intellij-html-description": "describes user defined filters describing how skaffold should treat objects/fields during rendering."
         }
       },
       "preferredOrder": [
@@ -3544,6 +3583,7 @@
         "test",
         "deploy",
         "portForward",
+        "transform",
         "profiles"
       ],
       "additionalProperties": false,

diff --git a/pkg/skaffold/config/options.go b/pkg/skaffold/config/options.go
@@ -46,12 +46,13 @@ type SkaffoldOptions struct {
 	// TODO(https://github.com/GoogleContainerTools/skaffold/issues/3668):
 	// remove minikubeProfile from here and instead detect it by matching the
 	// kubecontext API Server to minikube profiles
-	MinikubeProfile string
-	Namespace       string
-	RenderOutput    string
-	RepoCacheDir    string
-	Trigger         string
-	User            string
+	MinikubeProfile    string
+	Namespace          string
+	RenderOutput       string
+	RepoCacheDir       string
+	Trigger            string
+	User               string
+	TransformRulesFile string
 
 	ConfigurationFilter []string
 	CustomLabels        []string

diff --git a/pkg/skaffold/deploy/deploy_problems_test.go b/pkg/skaffold/deploy/deploy_problems_test.go
@@ -108,18 +108,21 @@ type mockConfig struct {
 	kubeContext string
 }
 
-func (m mockConfig) MinikubeProfile() string                           { return m.minikube }
-func (m mockConfig) GetPipelines() []latestV1.Pipeline                 { return []latestV1.Pipeline{} }
-func (m mockConfig) GetWorkingDir() string                             { return "" }
-func (m mockConfig) GetNamespace() string                              { return "" }
-func (m mockConfig) GlobalConfig() string                              { return "" }
-func (m mockConfig) ConfigurationFile() string                         { return "" }
-func (m mockConfig) DefaultRepo() *string                              { return &m.minikube }
-func (m mockConfig) MultiLevelRepo() *bool                             { return nil }
-func (m mockConfig) SkipRender() bool                                  { return true }
-func (m mockConfig) Prune() bool                                       { return true }
-func (m mockConfig) ContainerDebugging() bool                          { return false }
-func (m mockConfig) GetKubeContext() string                            { return m.kubeContext }
-func (m mockConfig) GetInsecureRegistries() map[string]bool            { return map[string]bool{} }
-func (m mockConfig) Mode() config.RunMode                              { return config.RunModes.Dev }
-func (m mockConfig) TransformableAllowList() []latestV1.ResourceFilter { return nil }
+func (m mockConfig) MinikubeProfile() string                       { return m.minikube }
+func (m mockConfig) GetPipelines() []latestV1.Pipeline             { return []latestV1.Pipeline{} }
+func (m mockConfig) GetWorkingDir() string                         { return "" }
+func (m mockConfig) GetNamespace() string                          { return "" }
+func (m mockConfig) GlobalConfig() string                          { return "" }
+func (m mockConfig) ConfigurationFile() string                     { return "" }
+func (m mockConfig) DefaultRepo() *string                          { return &m.minikube }
+func (m mockConfig) MultiLevelRepo() *bool                         { return nil }
+func (m mockConfig) SkipRender() bool                              { return true }
+func (m mockConfig) Prune() bool                                   { return true }
+func (m mockConfig) ContainerDebugging() bool                      { return false }
+func (m mockConfig) GetKubeContext() string                        { return m.kubeContext }
+func (m mockConfig) GetInsecureRegistries() map[string]bool        { return map[string]bool{} }
+func (m mockConfig) Mode() config.RunMode                          { return config.RunModes.Dev }
+func (m mockConfig) TransformAllowList() []latestV1.ResourceFilter { return nil }
+func (m mockConfig) TransformDenyList() []latestV1.ResourceFilter  { return nil }
+func (m mockConfig) GetTransformAllowListFile() string             { return "" }
+func (m mockConfig) GetTransformDenyListFile() string              { return "" }
diff --git a/pkg/skaffold/deploy/kpt/kpt.go b/pkg/skaffold/deploy/kpt/kpt.go
@@ -30,6 +30,7 @@ import (
 
 	"golang.org/x/mod/semver"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
+	apimachinery "k8s.io/apimachinery/pkg/runtime/schema"
 	k8syaml "sigs.k8s.io/yaml"
 
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/access"
@@ -100,6 +101,9 @@ type Deployer struct {
 	namespace          string
 
 	namespaces *[]string
+
+	transformableAllowlist map[apimachinery.GroupKind]latestV1.ResourceFilter
+	transformableDenylist  map[apimachinery.GroupKind]latestV1.ResourceFilter
 }
 
 type Config interface {
@@ -110,32 +114,38 @@ type Config interface {
 }
 
 // NewDeployer generates a new Deployer object contains the kptDeploy schema.
-func NewDeployer(cfg Config, labeller *label.DefaultLabeller, d *latestV1.KptDeploy) *Deployer {
+func NewDeployer(cfg Config, labeller *label.DefaultLabeller, d *latestV1.KptDeploy) (*Deployer, error) {
 	podSelector := kubernetes.NewImageList()
 	kubectl := pkgkubectl.NewCLI(cfg, cfg.GetKubeNamespace())
 	namespaces, err := deployutil.GetAllPodNamespaces(cfg.GetNamespace(), cfg.GetPipelines())
 	if err != nil {
 		olog.Entry(context.TODO()).Warn("unable to parse namespaces - deploy might not work correctly!")
 	}
 	logger := component.NewLogger(cfg, kubectl, podSelector, &namespaces)
-	return &Deployer{
-		KptDeploy:          d,
-		podSelector:        podSelector,
-		namespaces:         &namespaces,
-		accessor:           component.NewAccessor(cfg, cfg.GetKubeContext(), kubectl, podSelector, labeller, &namespaces),
-		debugger:           component.NewDebugger(cfg.Mode(), podSelector, &namespaces, cfg.GetKubeContext()),
-		imageLoader:        component.NewImageLoader(cfg, kubectl),
-		logger:             logger,
-		statusMonitor:      component.NewMonitor(cfg, cfg.GetKubeContext(), labeller, &namespaces),
-		syncer:             component.NewSyncer(kubectl, &namespaces, logger.GetFormatter()),
-		insecureRegistries: cfg.GetInsecureRegistries(),
-		labels:             labeller.Labels(),
-		globalConfig:       cfg.GlobalConfig(),
-		hasKustomization:   hasKustomization,
-		kubeContext:        cfg.GetKubeContext(),
-		kubeConfig:         cfg.GetKubeConfig(),
-		namespace:          cfg.GetKubeNamespace(),
+	transformableAllowlist, transformableDenylist, err := deployutil.ConsolidateTransformConfiguration(cfg)
+	if err != nil {
+		return nil, err
 	}
+	return &Deployer{
+		KptDeploy:              d,
+		podSelector:            podSelector,
+		namespaces:             &namespaces,
+		accessor:               component.NewAccessor(cfg, cfg.GetKubeContext(), kubectl, podSelector, labeller, &namespaces),
+		debugger:               component.NewDebugger(cfg.Mode(), podSelector, &namespaces, cfg.GetKubeContext()),
+		imageLoader:            component.NewImageLoader(cfg, kubectl),
+		logger:                 logger,
+		statusMonitor:          component.NewMonitor(cfg, cfg.GetKubeContext(), labeller, &namespaces),
+		syncer:                 component.NewSyncer(kubectl, &namespaces, logger.GetFormatter()),
+		insecureRegistries:     cfg.GetInsecureRegistries(),
+		labels:                 labeller.Labels(),
+		globalConfig:           cfg.GlobalConfig(),
+		hasKustomization:       hasKustomization,
+		kubeContext:            cfg.GetKubeContext(),
+		kubeConfig:             cfg.GetKubeConfig(),
+		namespace:              cfg.GetKubeNamespace(),
+		transformableAllowlist: transformableAllowlist,
+		transformableDenylist:  transformableDenylist,
+	}, nil
 }
 
 func (k *Deployer) trackNamespaces(namespaces []string) {
@@ -475,12 +485,12 @@ func (k *Deployer) renderManifests(ctx context.Context, builds []graph.Artifact)
 		return nil, fmt.Errorf("excluding kpt functions from manifests: %w", err)
 	}
 	if k.originalImages == nil {
-		k.originalImages, err = manifests.GetImages()
+		k.originalImages, err = manifests.GetImages(manifest.NewResourceSelectorImages(k.transformableAllowlist, k.transformableDenylist))
 		if err != nil {
 			return nil, err
 		}
 	}
-	manifests, err = manifests.ReplaceImages(ctx, builds)
+	manifests, err = manifests.ReplaceImages(ctx, builds, manifest.NewResourceSelectorImages(k.transformableAllowlist, k.transformableDenylist))
 	if err != nil {
 		return nil, fmt.Errorf("replacing images in manifests: %w", err)
 	}
@@ -489,7 +499,7 @@ func (k *Deployer) renderManifests(ctx context.Context, builds []graph.Artifact)
 		return nil, err
 	}
 
-	return manifests.SetLabels(k.labels)
+	return manifests.SetLabels(k.labels, manifest.NewResourceSelectorLabels(k.transformableAllowlist, k.transformableDenylist))
 }
 
 func sink(ctx context.Context, buf []byte, sinkDir string) error {