Skip to content

Commit

Permalink
runtime validation for docker network stack sharing
Browse files Browse the repository at this point in the history
  • Loading branch information
zedfmario committed Dec 4, 2020
1 parent df3e6b3 commit f7078b3
Show file tree
Hide file tree
Showing 6 changed files with 199 additions and 0 deletions.
4 changes: 4 additions & 0 deletions cmd/skaffold/app/cmd/runner.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down
3 changes: 3 additions & 0 deletions examples/getting-started/skaffold.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,9 @@ kind: Config
build:
artifacts:
- image: skaffold-example
context: .
docker:
network: "container:alpine"
deploy:
kubectl:
manifests:
Expand Down
1 change: 1 addition & 0 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ require (
github.com/go-git/go-git/v5 v5.0.0
github.com/golang/glog v0.0.0-20160126235308-23def4e6c14b
github.com/golang/groupcache v0.0.0-20200121045136-8c9f03a8e57e
github.com/golang/mock v1.4.4
github.com/golang/protobuf v1.4.3
github.com/google/go-cmp v0.5.2
github.com/google/go-containerregistry v0.1.4
Expand Down
1 change: 1 addition & 0 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -405,6 +405,7 @@ github.com/golang/mock v1.3.1/go.mod h1:sBzyDLLjw3U8JLTeZvSv8jJB+tU5PVekmnlKIyFU
github.com/golang/mock v1.4.0/go.mod h1:UOMv5ysSaYNkG+OFQykRIcU/QvvxJf3p21QfJ2Bt3cw=
github.com/golang/mock v1.4.1/go.mod h1:UOMv5ysSaYNkG+OFQykRIcU/QvvxJf3p21QfJ2Bt3cw=
github.com/golang/mock v1.4.3/go.mod h1:UOMv5ysSaYNkG+OFQykRIcU/QvvxJf3p21QfJ2Bt3cw=
github.com/golang/mock v1.4.4 h1:l75CXGRSwbaYNpl/Z2X1XIIAMSCquvXgpVZDhwEIJsc=
github.com/golang/mock v1.4.4/go.mod h1:l3mdAwkq5BuhzHwde/uurv3sEJeZMXNpwsxVWU71h+4=
github.com/golang/protobuf v0.0.0-20161109072736-4bd1920723d7/go.mod h1:6lQm79b+lXiMfvg/cZm0SGofjICqVBUtrP5yJMmIC1U=
github.com/golang/protobuf v1.2.0/go.mod h1:6lQm79b+lXiMfvg/cZm0SGofjICqVBUtrP5yJMmIC1U=
Expand Down
72 changes: 72 additions & 0 deletions pkg/skaffold/schema/validation/validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,19 @@ limitations under the License.
package validation

import (
"context"
"fmt"
"reflect"
"regexp"
"strings"
"time"

"github.com/docker/docker/api/types"
"github.com/docker/docker/client"

"github.com/GoogleContainerTools/skaffold/pkg/skaffold/build/misc"
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/docker"
"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"
Expand Down Expand Up @@ -60,6 +66,35 @@ 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 {
apiClient, err := docker.NewAPIClient(runCtx)
if err != nil {
return err
}
client := apiClient.RawClient()

var errs []error
errs = append(errs, ProcessWithDockerClient(config, client)...)
if len(errs) == 0 {
return nil
}
var messages []string
for _, err := range errs {
messages = append(messages, err.Error())
}
return fmt.Errorf(strings.Join(messages, " | "))
}

// ProcessWithDockerClient checks if the Skaffold pipeline is valid when a client.CommonAPIClient is required.
// It returns all encountered errors as a concatenated string.
// Injecting `client` –a client.CommonAPIClient– for make it testable
func ProcessWithDockerClient(config *latest.SkaffoldConfig, client client.CommonAPIClient) (errs []error) {
errs = append(errs, validateDockerNetworkContainerExists(config.Build.Artifacts, client)...)
return
}

// validateTaggingPolicy checks that the tagging policy is valid in combination with other options.
func validateTaggingPolicy(bc latest.BuildConfig) (errs []error) {
if bc.LocalBuild != nil {
Expand Down Expand Up @@ -199,6 +234,43 @@ func validateDockerNetworkMode(artifacts []*latest.Artifact) (errs []error) {
return
}

// Validates that a Docker Container with a Network Mode "container:<id|name>" points to an actually running container
func validateDockerNetworkContainerExists(artifacts []*latest.Artifact, client client.CommonAPIClient) []error {
ctx, cancel := context.WithDeadline(context.Background(), time.Now().Add(5*time.Second))
defer cancel()

var errs []error
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, fmt.Errorf("retrieving docker containers list: %s", err.Error()))
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
}
}
}
errs = append(errs, fmt.Errorf("container '%s' not found. Required by image '%s' for docker network stack sharing", id, a.ImageName))
}
}
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 {
Expand Down
118 changes: 118 additions & 0 deletions pkg/skaffold/schema/validation/validation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,12 @@ package validation

import (
"fmt"
"strings"
"testing"

"github.com/buildpacks/pack/testmocks"
"github.com/docker/docker/api/types"
"github.com/golang/mock/gomock"
"github.com/google/go-cmp/cmp"

"github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/latest"
Expand Down Expand Up @@ -435,6 +439,120 @@ func TestValidateNetworkMode(t *testing.T) {
}
}

func TestValidateNetworkModeDockerContainerExists(t *testing.T) {
ctrl := gomock.NewController(t)
defer ctrl.Finish()

tests := []struct {
description string
artifacts []*latest.Artifact
clientResponse []types.Container
shouldErr bool
}{
{
description: "no running containers",
artifacts: []*latest.Artifact{
{
ImageName: "image/container",
ArtifactType: latest.ArtifactType{
DockerArtifact: &latest.DockerArtifact{
NetworkMode: "Container:foo",
},
},
},
},
clientResponse: []types.Container{},
shouldErr: true,
},
{
description: "not matching running containers",
artifacts: []*latest.Artifact{
{
ImageName: "image/container",
ArtifactType: latest.ArtifactType{
DockerArtifact: &latest.DockerArtifact{
NetworkMode: "Container:foo",
},
},
},
},
clientResponse: []types.Container{
{
ID: "not-foo",
Names: []string{"/bar"},
},
},
shouldErr: true,
},
{
description: "existing running container referenced by id",
artifacts: []*latest.Artifact{
{
ImageName: "image/container",
ArtifactType: latest.ArtifactType{
DockerArtifact: &latest.DockerArtifact{
NetworkMode: "Container:foo",
},
},
},
},
clientResponse: []types.Container{
{
ID: "foo",
},
},
},
{
description: "existing running container referenced by name",
artifacts: []*latest.Artifact{
{
ImageName: "image/container",
ArtifactType: latest.ArtifactType{
DockerArtifact: &latest.DockerArtifact{
NetworkMode: "Container:foo",
},
},
},
},
clientResponse: []types.Container{
{
ID: "no-foo",
Names: []string{"/foo"},
},
},
},
}
for _, test := range tests {
testutil.Run(t, test.description, func(t *testutil.T) {
// disable yamltags validation
t.Override(&validateYamltags, func(interface{}) error { return nil })

client := testmocks.NewMockCommonAPIClient(ctrl)
client.EXPECT().ContainerList(gomock.Any(), gomock.Any()).Return(test.clientResponse, nil)

errs := ProcessWithDockerClient(
&latest.SkaffoldConfig{
Pipeline: latest.Pipeline{
Build: latest.BuildConfig{
Artifacts: test.artifacts,
},
},
}, client)

var err error
if len(errs) != 0 {
var messages []string
for _, e := range errs {
messages = append(messages, e.Error())
}
err = fmt.Errorf(strings.Join(messages, " | "))
}

t.CheckError(test.shouldErr, err)
})
}
}

func TestValidateSyncRules(t *testing.T) {
tests := []struct {
description string
Expand Down

0 comments on commit f7078b3

Please sign in to comment.