Merge branch 'master' into add_skaffold_dev_loop_events

integration/rpc_test.go

@@ -116,13 +116,18 @@ func TestEventsRPC(t *testing.T) {
 		switch entry.Event.GetEventType().(type) {
 		case *proto.Event_MetaEvent:
 			metaEntries++
+			t.Logf("meta event %d: %v", metaEntries, entry.Event)
 		case *proto.Event_BuildEvent:
 			buildEntries++
+			t.Logf("build event %d: %v", buildEntries, entry.Event)
 		case *proto.Event_DeployEvent:
 			deployEntries++
+      t.Logf("deploy event %d: %v", deployEntries, entry.Event)
 		case *proto.Event_DevLoopEvent:
 			devLoopEntries++
+      t.Logf("deploy event %d: %v", deployEntries, entry.Event)
 		default:
+			t.Logf("unknown event: %v", entry.Event)
 		}
 	}
 	// make sure we have exactly 1 meta entry, 2 deploy entries and 2 build entries and 2 devLoopEntries

pkg/skaffold/debug/cnb.go

@@ -0,0 +1,144 @@
+/*
+Copyright 2020 The Skaffold Authors
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package debug
+
+import (
+	"encoding/json"
+	"fmt"
+
+	cnb "github.com/buildpacks/lifecycle"
+	shell "github.com/kballard/go-shellquote"
+	"github.com/sirupsen/logrus"
+	v1 "k8s.io/api/core/v1"
+)
+
+func init() {
+	// the CNB's launcher just launches the command
+	entrypointLaunchers = append(entrypointLaunchers, "/cnb/lifecycle/launcher")
+}
+
+// updateForCNBImage normalizes an imageConfiguration from a Cloud Native Buildpacks-created image
+// prior to handing off to another transformer.  This transformer is usually the `debug` container
+// transform process.  CNB images have their entrypoint set to the CNB launcher, and the image command
+// describe the launch parameters.  After the transformer, updateForCNBImage rewrites the changed
+// image command back to the form expected by the CNB launcher.
+//
+// The CNB launcher supports three types of launches:
+//
+//   1. _predefined processes_ are named sets of command+arguments.  There are two types:
+//      - direct: these are passed uninterpreted to os.exec; the command is resolved in PATH
+//      - script: the command is treated as a shell script and passed to `sh -c`, the remaining
+//        arguments are added to the shell, and so available as positional arguments.
+//        For example: `sh -c 'echo $0 $1 $2 $3' arg0 arg1 arg2 arg3` => `arg0 arg1 arg2 arg3`.
+//        (https://github.com/buildpacks/lifecycle/issues/218#issuecomment-567091462).
+//   2. _direct execs_ where the container's arg[0] == `--` are treated like direct processes
+//   3. _shell scripts_ where the container's arg[0] is the script and treated are like indirect processes.
+//
+// A key point is that the script-style launches allow support references to environment variables.
+// So we find the command line to be executed, whether that is a script or a direct, and turn it into
+// a normal command-line.  But we also return a rewriter to transform the the command-line back to
+// the original form.
+func updateForCNBImage(container *v1.Container, ic imageConfiguration, transformer func(container *v1.Container, ic imageConfiguration) (ContainerDebugConfiguration, string, error)) (ContainerDebugConfiguration, string, error) {
+	// The build metadata isn't absolutely required as the image args could be
+	// a command line (e.g., `python xxx`) but it likely indicates the
+	// image was built with an older lifecycle.
+	// buildpacks/lifecycle 0.6.0 now embeds processes into special image label
+	metadataJSON, found := ic.labels["io.buildpacks.build.metadata"]
+	if !found {
+		return ContainerDebugConfiguration{}, "", fmt.Errorf("image is missing buildpacks metadata; perhaps built with older lifecycle?")
+	}
+	m := cnb.BuildMetadata{}
+	if err := json.Unmarshal([]byte(metadataJSON), &m); err != nil {
+		return ContainerDebugConfiguration{}, "", fmt.Errorf("unable to parse image buildpacks metadata")
+	}
+	if len(m.Processes) == 0 {
+		return ContainerDebugConfiguration{}, "", fmt.Errorf("buildpacks metadata has no processes")
+	}
+
+	// the buildpacks launcher is retained as the entrypoint
+	ic, rewriter := adjustCommandLine(m, ic)
+	c, img, err := transformer(container, ic)
+	if err == nil && rewriter != nil {
+		container.Args = rewriter(container.Args)
+	}
+	return c, img, err
+}
+
+// adjustCommandLine resolves the launch process and then rewrites the command-line to be
+// in a form suitable for the normal `skaffold debug` transformations.  It returns an
+// amended configuration with a function to re-transform the command-line to the form
+// expected by the launcher.
+func adjustCommandLine(m cnb.BuildMetadata, ic imageConfiguration) (imageConfiguration, func([]string) []string) {
+	// direct exec
+	if len(ic.arguments) > 0 && ic.arguments[0] == "--" {
+		// strip and restore the "--"
+		ic.arguments = ic.arguments[1:]
+		return ic, func(transformed []string) []string {
+			return append([]string{"--"}, transformed...)
+		}
+	}
+
+	processType := "web" // default buildpacks process type
+	// the launcher accepts the first argument as a process type
+	if len(ic.arguments) == 1 {
+		processType = ic.arguments[0]
+	} else if value := ic.env["CNB_PROCESS_TYPE"]; len(value) > 0 {
+		processType = value
+	}
+
+	for _, p := range m.Processes {
+		if p.Type == processType {
+			if p.Direct {
+				// p.Command is the command and p.Args are the arguments
+				ic.arguments = append([]string{p.Command}, p.Args...)
+				return ic, func(transformed []string) []string {
+					return append([]string{"--"}, transformed...)
+				}
+			}
+			// Script type: split p.Command, pass it through the transformer, and then reassemble in the rewriter.
+			if args, err := shell.Split(p.Command); err == nil {
+				ic.arguments = args
+			} else {
+				ic.arguments = []string{p.Command}
+			}
+			return ic, func(transformed []string) []string {
+				// reassemble back into a script with arguments
+				return append([]string{shJoin(transformed)}, p.Args...)
+			}
+		}
+	}
+
+	if len(ic.arguments) == 0 {
+		// indicates an image mis-configuration as we should have resolved the the
+		// CNB_PROCESS_TYPE (if specified) or `web`.
+		logrus.Warnf("no CNB launch found for %s/%s", ic.artifact, processType)
+		return ic, nil
+	}
+
+	// ic.arguments[0] is a shell script:  split it, pass it through the transformer, and then reassemble in the rewriter.
+	// If it can't be split, then we fall through and return it untouched, to be handled by the normal debug process.
+	var rewriter func(transformed []string) []string
+	if args, err := shell.Split(ic.arguments[0]); err == nil {
+		remnants := ic.arguments[1:]
+		ic.arguments = args
+		rewriter = func(transformed []string) []string {
+			// reassemble back into a script with arguments
+			return append([]string{shJoin(transformed)}, remnants...)
+		}
+	}
+	return ic, rewriter
+}

pkg/skaffold/debug/cnb_test.go

@@ -0,0 +1,124 @@
+/*
+Copyright 2020 The Skaffold Authors
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package debug
+
+import (
+	"encoding/json"
+	"testing"
+
+	cnb "github.com/buildpacks/lifecycle"
+	"github.com/buildpacks/lifecycle/launch"
+	v1 "k8s.io/api/core/v1"
+
+	"github.com/GoogleContainerTools/skaffold/testutil"
+)
+
+func TestUpdateForCNBImage(t *testing.T) {
+	// metadata with default process type
+	md := cnb.BuildMetadata{Processes: []launch.Process{
+		{Type: "web", Command: "webProcess", Args: []string{"webArg1", "webArg2"}},
+		{Type: "diag", Command: "diagProcess"},
+		{Type: "direct", Command: "command", Args: []string{"cmdArg1"}, Direct: true},
+	}}
+	mdMarshalled, _ := json.Marshal(&md)
+	mdJSON := string(mdMarshalled)
+	// metadata with no default process type
+	mdnd := cnb.BuildMetadata{Processes: []launch.Process{
+		{Type: "diag", Command: "diagProcess"},
+		{Type: "direct", Command: "command", Args: []string{"cmdArg1"}, Direct: true},
+	}}
+	mdndMarshalled, _ := json.Marshal(&mdnd)
+	mdndJSON := string(mdndMarshalled)
+
+	tests := []struct {
+		description string
+		input       imageConfiguration
+		shouldErr   bool
+		expected    v1.Container
+	}{
+		{
+			description: "error when missing build.metadata",
+			input:       imageConfiguration{entrypoint: []string{"/cnb/lifecycle/launcher"}},
+			shouldErr:   true,
+		},
+		{
+			description: "error when build.metadata missing processes",
+			input:       imageConfiguration{entrypoint: []string{"/cnb/lifecycle/launcher"}, labels: map[string]string{"io.buildpacks.build.metadata": "{}"}},
+			shouldErr:   true,
+		},
+		{
+			description: "direct command-lines are rewritten as direct command-lines",
+			input:       imageConfiguration{entrypoint: []string{"/cnb/lifecycle/launcher"}, arguments: []string{"--", "web", "arg1", "arg2"}, labels: map[string]string{"io.buildpacks.build.metadata": mdJSON}},
+			shouldErr:   false,
+			expected:    v1.Container{Args: []string{"--", "web", "arg1", "arg2"}},
+		},
+		{
+			description: "defaults to web process when no process type",
+			input:       imageConfiguration{entrypoint: []string{"/cnb/lifecycle/launcher"}, labels: map[string]string{"io.buildpacks.build.metadata": mdJSON}},
+			shouldErr:   false,
+			expected:    v1.Container{Args: []string{"webProcess", "webArg1", "webArg2"}},
+		},
+		{
+			description: "resolves to default 'web' process",
+			input:       imageConfiguration{entrypoint: []string{"/cnb/lifecycle/launcher"}, labels: map[string]string{"io.buildpacks.build.metadata": mdJSON}},
+			shouldErr:   false,
+			expected:    v1.Container{Args: []string{"webProcess", "webArg1", "webArg2"}},
+		},
+		{
+			description: "CNB_PROCESS_TYPE=web",
+			input:       imageConfiguration{entrypoint: []string{"/cnb/lifecycle/launcher"}, env: map[string]string{"CNB_PROCESS_TYPE": "web"}, labels: map[string]string{"io.buildpacks.build.metadata": mdJSON}},
+			shouldErr:   false,
+			expected:    v1.Container{Args: []string{"webProcess", "webArg1", "webArg2"}},
+		},
+		{
+			description: "CNB_PROCESS_TYPE=diag",
+			input:       imageConfiguration{entrypoint: []string{"/cnb/lifecycle/launcher"}, env: map[string]string{"CNB_PROCESS_TYPE": "diag"}, labels: map[string]string{"io.buildpacks.build.metadata": mdJSON}},
+			shouldErr:   false,
+			expected:    v1.Container{Args: []string{"diagProcess"}},
+		},
+		{
+			description: "CNB_PROCESS_TYPE=direct",
+			input:       imageConfiguration{entrypoint: []string{"/cnb/lifecycle/launcher"}, env: map[string]string{"CNB_PROCESS_TYPE": "direct"}, labels: map[string]string{"io.buildpacks.build.metadata": mdJSON}},
+			shouldErr:   false,
+			expected:    v1.Container{Args: []string{"--", "command", "cmdArg1"}},
+		},
+		{
+			description: "script command-line",
+			input:       imageConfiguration{entrypoint: []string{"/cnb/lifecycle/launcher"}, arguments: []string{"python main.py"}, labels: map[string]string{"io.buildpacks.build.metadata": mdJSON}},
+			shouldErr:   false,
+			expected:    v1.Container{Args: []string{"python main.py"}},
+		},
+		{
+			description: "no process and no args",
+			input:       imageConfiguration{entrypoint: []string{"/cnb/lifecycle/launcher"}, labels: map[string]string{"io.buildpacks.build.metadata": mdndJSON}},
+			shouldErr:   false,
+			expected:    v1.Container{},
+		},
+	}
+	for _, test := range tests {
+		testutil.Run(t, test.description, func(t *testutil.T) {
+			dummyTransform := func(c *v1.Container, ic imageConfiguration) (ContainerDebugConfiguration, string, error) {
+				c.Args = ic.arguments
+				return ContainerDebugConfiguration{}, "", nil
+			}
+
+			copy := v1.Container{}
+			_, _, err := updateForCNBImage(&copy, test.input, dummyTransform)
+			t.CheckErrorAndDeepEqual(test.shouldErr, err, test.expected, copy)
+		})
+	}
+}

pkg/skaffold/debug/transform.go

@@ -117,8 +117,27 @@ const (
 	DebugConfigAnnotation = "debug.cloud.google.com/config"
 )
 
+// containerTransforms are the set of configured transformers
 var containerTransforms []containerTransformer
 
+// entrypointLaunchers is a list of known entrypoints that effectively just launches the container image's CMD
+// as a command-line.  These entrypoints are ignored.
+var entrypointLaunchers []string
+
+// isEntrypointLauncher checks if the given entrypoint is a known entrypoint launcher,
+// meaning an entrypoint that treats the image's CMD as a command-line.
+func isEntrypointLauncher(entrypoint []string) bool {
+	if len(entrypoint) != 1 {
+		return false
+	}
+	for _, knownEntrypoints := range entrypointLaunchers {
+		if knownEntrypoints == entrypoint[0] {
+			return true
+		}
+	}
+	return false
+}
+
 // transformManifest attempts to configure a manifest for debugging.
 // Returns true if changed, false otherwise.
 func transformManifest(obj runtime.Object, retrieveImageConfiguration configurationRetriever) bool {
@@ -300,6 +319,18 @@ func transformContainer(container *v1.Container, config imageConfiguration, port
 		config.arguments = container.Args
 	}
 
+	// Buildpack-generated images require special handling
+	if _, found := config.labels["io.buildpacks.stack.id"]; found && len(config.entrypoint) > 0 && config.entrypoint[0] == "/cnb/lifecycle/launcher" {
+		next := func(container *v1.Container, config imageConfiguration) (ContainerDebugConfiguration, string, error) {
+			return performContainerTransform(container, config, portAlloc)
+		}
+		return updateForCNBImage(container, config, next)
+	}
+
+	return performContainerTransform(container, config, portAlloc)
+}
+
+func performContainerTransform(container *v1.Container, config imageConfiguration, portAlloc portAllocator) (ContainerDebugConfiguration, string, error) {
 	for _, transform := range containerTransforms {
 		if transform.IsApplicable(config) {
 			return transform.Apply(container, config, portAlloc)
@@ -383,3 +414,21 @@ func setEnvVar(entries []v1.EnvVar, varName, value string) []v1.EnvVar {
 	}
 	return append(entries, entry)
 }
+
+// shJoin joins the arguments into a quoted form suitable to pass to `sh -c`.
+// Necessary as github.com/kballard/go-shellquote's `Join` quotes `$`.
+func shJoin(args []string) string {
+	result := ""
+	for i, arg := range args {
+		if i > 0 {
+			result += " "
+		}
+		if strings.ContainsAny(arg, " \t\r\n\"") {
+			arg := strings.ReplaceAll(arg, `"`, `\"`)
+			result += `"` + arg + `"`
+		} else {
+			result += arg
+		}
+	}
+	return result
+}