enabling using another container's network stack on build process (#5088

)

* enabling using another container's network stack on build process

Building docker images in a Jenkins-like environment running on a AWS EKS cluster secured with `kube2iam` requires the running pod to include an annotation. In our particular case, we share the docker daemon from the node with the running pod by mounting the socket as a volume. When `skaffold` builds a new image within this pod, it creates containers at node level due to the shared socket. Therefore, to have a brand new container handled by `kube2iam`, this container should reuse the pod's network stack –ergo the "pause" container's stack–.

Enabling the use of `container:<name|id>` network mode from `docker run` command. It allows a build process to reuse another container's stack.
> Described in [here](https://docs.docker.com/engine/reference/run/#network-settings)

* Adapting REGEXP to docker requirements

According to the `docker` CLI, when running a container with a wrong name, the following output is shown:
```
> docker run -it --name '$$' alpine
docker: Error response from daemon: Invalid container name ($$), only [a-zA-Z0-9][a-zA-Z0-9_.-] are allowed.
```
Concatenating `container:` to the expected regular expression `[a-zA-Z0-9][a-zA-Z0-9_.-]` helps us rejecting wrong values.

* runtime validation for docker network stack sharing

* overriding docker local daemon in tests

* validation with run context unified

better testing allows to unify methods

* adding skaffold error handling

* removing unnecessary dependency

cmd/skaffold/app/cmd/runner.go

@@ -105,6 +105,10 @@ func runContext(opts config.SkaffoldOptions) (*runcontext.RunContext, *latest.Sk
 		return nil, nil, fmt.Errorf("getting run context: %w", err)
 	}
 
+	if err := validation.ProcessWithRunContext(config, runCtx); err != nil {
+		return nil, nil, fmt.Errorf("invalid skaffold config: %w", err)
+	}
+
 	return runCtx, config, nil
 }
 

cmd/skaffold/app/cmd/runner_test.go

@@ -86,8 +86,11 @@ func TestCreateNewRunner(t *testing.T) {
 	for _, test := range tests {
 		testutil.Run(t, test.description, func(t *testutil.T) {
 			t.Override(&docker.NewAPIClient, func(docker.Config) (docker.LocalDaemon, error) {
-				return nil, nil
+				return docker.NewLocalDaemon(&testutil.FakeAPIClient{
+					ErrVersion: true,
+				}, nil, false, nil), nil
 			})
+
 			t.Override(&update.GetLatestAndCurrentVersion, func() (semver.Version, semver.Version, error) {
 				return semver.Version{}, semver.Version{}, nil
 			})

docs/content/en/docs/references/api/grpc.md

@@ -812,6 +812,9 @@ For Cancelled Error code, use range 800 to 850.
 | BUILD_UNKNOWN_JIB_PLUGIN_TYPE | 119 | Build error indicating unknown Jib plugin type. Should be one of [maven, gradle] |
 | BUILD_JIB_GRADLE_DEP_ERR | 120 | Build error determining dependency for jib gradle project. |
 | BUILD_JIB_MAVEN_DEP_ERR | 121 | Build error determining dependency for jib gradle project. |
+| INIT_DOCKER_NETWORK_LISTING_CONTAINERS | 122 | Docker build error when listing containers. |
+| INIT_DOCKER_NETWORK_INVALID_CONTAINER_NAME | 123 | Docker build error indicating an invalid container name (or id). |
+| INIT_DOCKER_NETWORK_CONTAINER_DOES_NOT_EXIST | 124 | Docker build error indicating the container referenced does not exists in the docker context used. |
 | STATUSCHECK_IMAGE_PULL_ERR | 300 | Container image pull error |
 | STATUSCHECK_CONTAINER_CREATING | 301 | Container creating error |
 | STATUSCHECK_RUN_CONTAINER_ERR | 302 | Container run error |
@@ -910,6 +913,8 @@ Enum for Suggestion codes
 | FIX_CACHE_FROM_ARTIFACT_CONFIG | 109 | Fix `cacheFrom` config for given artifact and try again |
 | FIX_SKAFFOLD_CONFIG_DOCKERFILE | 110 | Fix `dockerfile` config for a given artifact and try again. |
 | FIX_JIB_PLUGIN_CONFIGURATION | 111 | Use a supported Jib plugin type |
+| FIX_DOCKER_NETWORK_CONTAINER_NAME | 112 | Docker build network invalid docker container name (or id). |
+| CHECK_DOCKER_NETWORK_CONTAINER_RUNNING | 113 | Docker build network container not existing in the current context. |
 | CHECK_CLUSTER_CONNECTION | 201 | Check cluster connection |
 | CHECK_MINIKUBE_STATUS | 202 | Check minikube status |
 | INSTALL_HELM | 203 | Install helm tool |

docs/content/en/schemas/v2beta11.json

@@ -992,11 +992,12 @@
         },
         "network": {
           "type": "string",
-          "description": "passed through to docker and overrides the network configuration of docker builder. If unset, use whatever is configured in the underlying docker daemon. Valid modes are `host`: use the host's networking stack. `bridge`: use the bridged network configuration. `none`: no networking in the container.",
-          "x-intellij-html-description": "passed through to docker and overrides the network configuration of docker builder. If unset, use whatever is configured in the underlying docker daemon. Valid modes are <code>host</code>: use the host's networking stack. <code>bridge</code>: use the bridged network configuration. <code>none</code>: no networking in the container.",
+          "description": "passed through to docker and overrides the network configuration of docker builder. If unset, use whatever is configured in the underlying docker daemon. Valid modes are `host`: use the host's networking stack. `bridge`: use the bridged network configuration. `container:<name|id>`: reuse another container's network stack. `none`: no networking in the container.",
+          "x-intellij-html-description": "passed through to docker and overrides the network configuration of docker builder. If unset, use whatever is configured in the underlying docker daemon. Valid modes are <code>host</code>: use the host's networking stack. <code>bridge</code>: use the bridged network configuration. <code>container:&lt;name|id&gt;</code>: reuse another container's network stack. <code>none</code>: no networking in the container.",
           "enum": [
             "host",
             "bridge",
+            "container:<name|id>",
             "none"
           ]
         },

examples/getting-started/skaffold.yaml

@@ -3,6 +3,11 @@ kind: Config
 build:
   artifacts:
   - image: skaffold-example
+    context: .
+    docker:
+      network: "container:alpine"
+  local:
+    push: false
 deploy:
   kubectl:
     manifests:

pkg/skaffold/schema/latest/config.go

@@ -1115,6 +1115,7 @@ type DockerArtifact struct {
 	// is configured in the underlying docker daemon. Valid modes are
 	// `host`: use the host's networking stack.
 	// `bridge`: use the bridged network configuration.
+	// `container:<name|id>`: reuse another container's network stack.
 	// `none`: no networking in the container.
 	NetworkMode string `yaml:"network,omitempty"`
 

pkg/skaffold/schema/validation/validation.go

@@ -17,16 +17,23 @@ limitations under the License.
 package validation
 
 import (
+	"context"
 	"fmt"
 	"reflect"
 	"regexp"
 	"strings"
+	"time"
+
+	"github.com/docker/docker/api/types"
 
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/build/misc"
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/docker"
+	sErrors "github.com/GoogleContainerTools/skaffold/pkg/skaffold/errors"
+	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/runner/runcontext"
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/latest"
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/util"
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/yamltags"
+	"github.com/GoogleContainerTools/skaffold/proto"
 )
 
 var (
@@ -60,6 +67,22 @@ func Process(config *latest.SkaffoldConfig) error {
 	return fmt.Errorf(strings.Join(messages, " | "))
 }
 
+// ProcessWithRunContext checks if the Skaffold pipeline is valid when a RunContext is required.
+// It returns all encountered errors as a concatenated string.
+func ProcessWithRunContext(config *latest.SkaffoldConfig, runCtx *runcontext.RunContext) error {
+	var errs []error
+	errs = append(errs, validateDockerNetworkContainerExists(config.Build.Artifacts, runCtx)...)
+
+	if len(errs) == 0 {
+		return nil
+	}
+	var messages []string
+	for _, err := range errs {
+		messages = append(messages, err.Error())
+	}
+	return fmt.Errorf(strings.Join(messages, " | "))
+}
+
 // validateTaggingPolicy checks that the tagging policy is valid in combination with other options.
 func validateTaggingPolicy(bc latest.BuildConfig) (errs []error) {
 	if bc.LocalBuild != nil {
@@ -179,7 +202,7 @@ func validateUniqueDependencyAliases(artifacts []*latest.Artifact) (errs []error
 	return
 }
 
-// validateDockerNetworkMode makes sure that networkMode is one of `bridge`, `none`, or `host` if set.
+// validateDockerNetworkMode makes sure that networkMode is one of `bridge`, `none`, `container:<name|id>`, or `host` if set.
 func validateDockerNetworkMode(artifacts []*latest.Artifact) (errs []error) {
 	for _, a := range artifacts {
 		if a.DockerArtifact == nil || a.DockerArtifact.NetworkMode == "" {
@@ -189,11 +212,92 @@ func validateDockerNetworkMode(artifacts []*latest.Artifact) (errs []error) {
 		if mode == "none" || mode == "bridge" || mode == "host" {
 			continue
 		}
-		errs = append(errs, fmt.Errorf("artifact %s has invalid networkMode '%s'", a.ImageName, mode))
+		containerRegExp := regexp.MustCompile("^container:[a-zA-Z0-9][a-zA-Z0-9_.-]*$")
+		if containerRegExp.MatchString(mode) {
+			continue
+		}
+
+		errMsg := fmt.Sprintf("artifact %s has invalid networkMode '%s'", a.ImageName, mode)
+		errs = append(errs, sErrors.NewError(fmt.Errorf(errMsg),
+			proto.ActionableErr{
+				Message: errMsg,
+				ErrCode: proto.StatusCode_INIT_DOCKER_NETWORK_INVALID_CONTAINER_NAME,
+				Suggestions: []*proto.Suggestion{
+					{
+						SuggestionCode: proto.SuggestionCode_FIX_DOCKER_NETWORK_CONTAINER_NAME,
+						Action:         "Please fix the docker network container name and try again",
+					},
+				},
+			}))
 	}
 	return
 }
 
+// Validates that a Docker Container with a Network Mode "container:<id|name>" points to an actually running container
+func validateDockerNetworkContainerExists(artifacts []*latest.Artifact, runCtx docker.Config) []error {
+	var errs []error
+	apiClient, err := docker.NewAPIClient(runCtx)
+	if err != nil {
+		errs = append(errs, err)
+		return errs
+	}
+
+	client := apiClient.RawClient()
+	ctx, cancel := context.WithDeadline(context.Background(), time.Now().Add(5*time.Second))
+	defer cancel()
+
+	for _, a := range artifacts {
+		if a.DockerArtifact == nil || a.DockerArtifact.NetworkMode == "" {
+			continue
+		}
+		mode := strings.ToLower(a.DockerArtifact.NetworkMode)
+		prefix := "container:"
+		if strings.HasPrefix(mode, prefix) {
+			id := strings.TrimPrefix(mode, prefix)
+			containers, err := client.ContainerList(ctx, types.ContainerListOptions{})
+			if err != nil {
+				errs = append(errs, sErrors.NewError(err,
+					proto.ActionableErr{
+						Message: "error retrieving docker containers list",
+						ErrCode: proto.StatusCode_INIT_DOCKER_NETWORK_LISTING_CONTAINERS,
+						Suggestions: []*proto.Suggestion{
+							{
+								SuggestionCode: proto.SuggestionCode_CHECK_DOCKER_RUNNING,
+								Action:         "Please check docker is running and try again",
+							},
+						},
+					}))
+				return errs
+			}
+			for _, c := range containers {
+				// Comparing ID seeking for <id>
+				if c.ID == id {
+					return errs
+				}
+				for _, name := range c.Names {
+					// c.Names come in form "/<name>"
+					if name == "/"+id {
+						return errs
+					}
+				}
+			}
+			errMsg := fmt.Sprintf("container '%s' not found, required by image '%s' for docker network stack sharing", id, a.ImageName)
+			errs = append(errs, sErrors.NewError(fmt.Errorf(errMsg),
+				proto.ActionableErr{
+					Message: errMsg,
+					ErrCode: proto.StatusCode_INIT_DOCKER_NETWORK_CONTAINER_DOES_NOT_EXIST,
+					Suggestions: []*proto.Suggestion{
+						{
+							SuggestionCode: proto.SuggestionCode_CHECK_DOCKER_NETWORK_CONTAINER_RUNNING,
+							Action:         "Please fix the docker network container name and try again.",
+						},
+					},
+				}))
+		}
+	}
+	return errs
+}
+
 // validateCustomDependencies makes sure that dependencies.ignore is only used in conjunction with dependencies.paths
 func validateCustomDependencies(artifacts []*latest.Artifact) (errs []error) {
 	for _, a := range artifacts {