Skip to content

Commit

Permalink
chore: remove validation for kpt version (#8425)
Browse files Browse the repository at this point in the history
* chore: remove kpt version validation

* chore: filter out config files in kpt fn render output
  • Loading branch information
ericzzzzzzz authored Feb 27, 2023
1 parent af94079 commit 086bdc1
Show file tree
Hide file tree
Showing 4 changed files with 210 additions and 67 deletions.
190 changes: 190 additions & 0 deletions integration/render_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1346,6 +1346,196 @@ spec:
}
}

func TestKptRender(t *testing.T) {
tests := []struct {
description string
args []string
config string
input map[string]string // file path => content
expectedOut string
}{
{
description: "simple kpt render",
args: []string{"--offline=true"},
config: `apiVersion: skaffold/v4beta2
kind: Config
metadata:
name: getting-started-kustomize
manifests:
kpt:
- apply-simple
`, input: map[string]string{"apply-simple/Kptfile": `apiVersion: kpt.dev/v1
kind: Kptfile
metadata:
name: apply-setters-simple
upstream:
type: git
git:
repo: https://github.com/GoogleContainerTools/kpt-functions-catalog
directory: /examples/apply-setters-simple
ref: apply-setters/v0.2.0
updateStrategy: resource-merge
upstreamLock:
type: git
git:
repo: https://github.com/GoogleContainerTools/kpt-functions-catalog
directory: /examples/apply-setters-simple
ref: apply-setters/v0.2.0
commit: 9b6ce80e355a53727d21b2b336f8da55e760e20ca
pipeline:
mutators:
- image: gcr.io/kpt-fn/apply-setters:v0.2
configMap:
nginx-replicas: 3
tag: 1.16.2
`, "apply-simple/resources.yaml": `apiVersion: apps/v1
kind: Deployment
metadata:
name: my-nginx
spec:
replicas: 4 # kpt-set: ${nginx-replicas}
selector:
matchLabels:
app: nginx
template:
metadata:
labels:
app: nginx
spec:
containers:
- name: nginx
image: "nginx:1.16.1" # kpt-set: nginx:${tag}
ports:
- protocol: TCP
containerPort: 80`},
expectedOut: `apiVersion: apps/v1
kind: Deployment
metadata:
name: my-nginx
spec:
replicas: 3
selector:
matchLabels:
app: nginx
template:
metadata:
labels:
app: nginx
spec:
containers:
- name: nginx
image: "nginx:1.16.2"
ports:
- protocol: TCP
containerPort: 80
`},
{
description: "kpt render with data config file",
args: []string{"--offline=true"},
config: `apiVersion: skaffold/v4beta2
kind: Config
metadata:
name: getting-started-kustomize
manifests:
kpt:
- set-annotations
`,
input: map[string]string{"set-annotations/Kptfile": `apiVersion: kpt.dev/v1
kind: Kptfile
metadata:
name: example
pipeline:
mutators:
- image: gcr.io/kpt-fn/set-annotations:v0.1.4
configPath: fn-config.yaml`,
"set-annotations/fn-config.yaml": `apiVersion: fn.kpt.dev/v1alpha1
kind: SetAnnotations
metadata: # kpt-merge: /my-func-config
name: my-func-config
annotations:
config.kubernetes.io/local-config: "true"
internal.kpt.dev/upstream-identifier: 'fn.kpt.dev|SetAnnotations|default|my-func-config'
annotations:
color: orange
fruit: apple
`,
"set-annotations/resources.yaml": `apiVersion: apps/v1
kind: Deployment
metadata:
name: my-nginx
spec:
replicas: 3 # kpt-set: ${nginx-replicas}
selector:
matchLabels:
app: nginx
template:
metadata:
labels:
app: nginx
spec:
containers:
- name: nginx
image: "nginx:1.16.1" # kpt-set: nginx:${tag}
ports:
- protocol: TCP
containerPort: 80`},
expectedOut: `apiVersion: apps/v1
kind: Deployment
metadata:
name: my-nginx
annotations:
color: orange
fruit: apple
spec:
replicas: 3
selector:
matchLabels:
app: nginx
template:
metadata:
annotations:
color: orange
fruit: apple
labels:
app: nginx
spec:
containers:
- name: nginx
image: "nginx:1.16.1"
ports:
- protocol: TCP
containerPort: 80
`},
}

for _, test := range tests {
testutil.Run(t, test.description, func(t *testutil.T) {
MarkIntegrationTest(t.T, CanRunWithoutGcp)
tmpDir := t.NewTempDir()
tmpDir.Write("skaffold.yaml", test.config)

for filePath, content := range test.input {
tmpDir.Write(filePath, content)
}

tmpDir.Chdir()
output := skaffold.Render(test.args...).RunOrFailOutput(t.T)
var out map[string]any
var ex map[string]any
err := yaml.Unmarshal(output, &out)
if err != nil {
return
}
err = yaml.Unmarshal([]byte(test.expectedOut), &ex)
if err != nil {
return
}

t.CheckDeepEqual(ex, out)
})
}
}

func TestHelmRenderWithImagesFlag(t *testing.T) {
tests := []struct {
description string
Expand Down
61 changes: 20 additions & 41 deletions pkg/skaffold/render/renderer/kpt/kpt.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,19 +17,22 @@ limitations under the License.
package kpt

import (
"bytes"
"context"
"fmt"
"io"
"os/exec"

"github.com/blang/semver"
apimachinery "k8s.io/apimachinery/pkg/runtime/schema"
"sigs.k8s.io/kustomize/kyaml/fn/framework"
"sigs.k8s.io/kustomize/kyaml/kio"
"sigs.k8s.io/kustomize/kyaml/yaml"

"github.com/GoogleContainerTools/skaffold/v2/pkg/skaffold/graph"
"github.com/GoogleContainerTools/skaffold/v2/pkg/skaffold/instrumentation"
"github.com/GoogleContainerTools/skaffold/v2/pkg/skaffold/kubernetes/manifest"
"github.com/GoogleContainerTools/skaffold/v2/pkg/skaffold/render"
"github.com/GoogleContainerTools/skaffold/v2/pkg/skaffold/render/generate"
"github.com/GoogleContainerTools/skaffold/v2/pkg/skaffold/render/kptfile"
rUtil "github.com/GoogleContainerTools/skaffold/v2/pkg/skaffold/render/renderer/util"
"github.com/GoogleContainerTools/skaffold/v2/pkg/skaffold/render/transform"
"github.com/GoogleContainerTools/skaffold/v2/pkg/skaffold/render/validate"
Expand All @@ -54,15 +57,6 @@ type Kpt struct {
transformDenylist map[apimachinery.GroupKind]latest.ResourceFilter
}

const (
DryFileName = "manifests.yaml"
)

var (
KptVersion = currentKptVersion
maxKptVersionAllowedForDeployer = "1.0.0-beta.24"
)

func New(cfg render.Config, rCfg latest.RenderConfig, hydrationDir string, labels map[string]string, configName string, ns string, manifestOverrides map[string]string) (*Kpt, error) {
transformAllowlist, transformDenylist, err := rUtil.ConsolidateTransformConfiguration(cfg)
if err != nil {
Expand Down Expand Up @@ -117,7 +111,21 @@ func (r *Kpt) Render(ctx context.Context, out io.Writer, builds []graph.Artifact
cmd := exec.Command("kpt", "fn", "render", p, "-o", "unwrap")

if buf, err := util.RunCmdOut(rCtx, cmd); err == nil {
manifestList.Append(buf)
reader := kio.ByteReader{Reader: bytes.NewBuffer(buf)}
b := bytes.NewBuffer([]byte{})
writer := kio.ByteWriter{Writer: b}
// Kpt fn render outputs Kptfile and Config data files content in result, we don't want them in our manifestList as these cannot be deployed to k8s cluster.
pipeline := kio.Pipeline{Filters: []kio.Filter{framework.ResourceMatcherFunc(func(node *yaml.RNode) bool {
meta, _ := node.GetMeta()
return node.GetKind() != kptfile.KptFileKind && meta.Annotations["config.kubernetes.io/local-config"] != "true"
})},
Inputs: []kio.Reader{&reader},
Outputs: []kio.Writer{writer},
}
if err := pipeline.Execute(); err != nil {
return ml, err
}
manifestList.Append(b.Bytes())
} else {
endTrace(instrumentation.TraceEndError(err))
// TODO(yuwenma): How to guide users when they face kpt error (may due to bad user config)?
Expand Down Expand Up @@ -156,32 +164,3 @@ func (r *Kpt) Render(ctx context.Context, out io.Writer, builds []graph.Artifact
ml.Add(r.configName, manifestList)
return ml, err
}

func CheckIsProperBinVersion(ctx context.Context) error {
maxAllowedVersion := semver.MustParse(maxKptVersionAllowedForDeployer)
version, err := KptVersion(ctx)
if err != nil {
return err
}

currentVersion, err := semver.ParseTolerant(version)
if err != nil {
return err
}

if currentVersion.GT(maxAllowedVersion) {
return fmt.Errorf("max allowed verion for Kpt renderer without Kpt deployer is %v, detected version is %v", maxKptVersionAllowedForDeployer, currentVersion)
}

return nil
}

func currentKptVersion(ctx context.Context) (string, error) {
cmd := exec.Command("kpt", "version")
b, err := util.RunCmdOut(ctx, cmd)
if err != nil {
return "", fmt.Errorf("kpt version command failed: %w", err)
}
version := string(b)
return version, nil
}
5 changes: 0 additions & 5 deletions pkg/skaffold/schema/validation/samples_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,15 +18,13 @@ package validation

import (
"bytes"
"context"
"fmt"
"os"
"path/filepath"
"strings"
"testing"

"github.com/GoogleContainerTools/skaffold/v2/pkg/skaffold/parser"
"github.com/GoogleContainerTools/skaffold/v2/pkg/skaffold/render/renderer/kpt"
"github.com/GoogleContainerTools/skaffold/v2/pkg/skaffold/schema"
"github.com/GoogleContainerTools/skaffold/v2/pkg/skaffold/schema/defaults"
"github.com/GoogleContainerTools/skaffold/v2/pkg/skaffold/schema/latest"
Expand Down Expand Up @@ -94,9 +92,6 @@ func checkSkaffoldConfig(t *testutil.T, yaml []byte) {
t.CheckNoError(err)
cfgs = append(cfgs, cfg)
}
t.Override(&kpt.KptVersion, func(_ context.Context) (string, error) {
return "1.0.0-beta.13", nil
})
err = Process(cfgs, Options{CheckDeploySource: false})
t.CheckNoError(err)
}
Expand Down
21 changes: 0 additions & 21 deletions pkg/skaffold/schema/validation/validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@ import (
"github.com/GoogleContainerTools/skaffold/v2/pkg/skaffold/output/log"
"github.com/GoogleContainerTools/skaffold/v2/pkg/skaffold/parser"
"github.com/GoogleContainerTools/skaffold/v2/pkg/skaffold/parser/configlocations"
"github.com/GoogleContainerTools/skaffold/v2/pkg/skaffold/render/renderer/kpt"
"github.com/GoogleContainerTools/skaffold/v2/pkg/skaffold/runner/runcontext"
"github.com/GoogleContainerTools/skaffold/v2/pkg/skaffold/schema/latest"
"github.com/GoogleContainerTools/skaffold/v2/pkg/skaffold/util"
Expand Down Expand Up @@ -87,7 +86,6 @@ func ProcessToErrorWithLocation(configs parser.SkaffoldConfigSet, validateConfig
errs = append(errs, validateTaggingPolicy(config, config.Build)...)
errs = append(errs, validateCustomTest(config, config.Test)...)
errs = append(errs, validateGCBConfig(config, config.Build)...)
errs = append(errs, validateKptRendererVersion(config, config.Deploy, config.Render)...)
}
errs = append(errs, validateArtifactDependencies(configs)...)
if validateConfig.CheckDeploySource {
Expand All @@ -100,25 +98,6 @@ func ProcessToErrorWithLocation(configs parser.SkaffoldConfigSet, validateConfig
return errs
}

func validateKptRendererVersion(cfg *parser.SkaffoldConfigEntry, dc latest.DeployConfig, rc latest.RenderConfig) (cfgErrs []ErrorWithLocation) {
if dc.KptDeploy != nil {
return
}

if rc.Kpt == nil && rc.Transform == nil && rc.Validate == nil { // no kpt renderer created
return
}

if err := kpt.CheckIsProperBinVersion(context.TODO()); err != nil {
cfgErrs = append(cfgErrs, ErrorWithLocation{
Error: err,
Location: cfg.YAMLInfos.LocateField(cfg, "Render"),
})
}

return
}

// Process checks if the Skaffold pipeline is valid and returns all encountered errors as a concatenated string
func Process(configs parser.SkaffoldConfigSet, validateConfig Options) error {
errs := ProcessToErrorWithLocation(configs, validateConfig)
Expand Down

0 comments on commit 086bdc1

Please sign in to comment.