diff --git a/README.md b/README.md index f3fa958..cbdaa42 100644 --- a/README.md +++ b/README.md @@ -9,7 +9,7 @@ Uses MutatingAdmissionWebhook in Kubernetes to inject sidecars into new deployme At Tumblr, we run some containers that have complicated sidecar setups. A kubernetes pod may run 5+ other containers, with some associated volumes and environment variables. It became clear quickly that keeping these sidecars in line would become an operational hassle; making sure every service uses the correct version of each dependency, updating global environment variable sets as configurations in our DCs change, etc. -To help solve this, we wrote the `k8s-sidecar-injector`. It is a small service that runs in each Kubernetes cluster, and listens to the Kubernetes API via webhooks. For each pod creation, the injector gets a (mutating admission) webhook, asking whether or not to allow the pod launch, and if allowed, what changes we would like to make to it. For pods that have special annotations on them (`injector.tumblr.com/request=some-sidecar-name`), we rewrite the pod configuration to include the containers, volumes, volume mounts, host aliases, init-containers and environment variables defined in the sidecar `some-sidecar-name`'s configuration. +To help solve this, we wrote the `k8s-sidecar-injector`. It is a small service that runs in each Kubernetes cluster, and listens to the Kubernetes API via webhooks. For each pod creation, the injector gets a (mutating admission) webhook, asking whether or not to allow the pod launch, and if allowed, what changes we would like to make to it. For pods that have special annotations on them (i.e. `injector.tumblr.com/request=logger:v1`), we rewrite the pod configuration to include the containers, volumes, volume mounts, host aliases, init-containers and environment variables defined in the sidecar `logger:v1`'s configuration. This enabled us to keep sane, centralized configuration for oft-used, but infrequently cared about configuration for our sidecars. @@ -23,10 +23,10 @@ See [/docs/deployment.md](/docs/deployment.md) to see what a sample deployment m # How it works -1. A pod is created. It has annotation `injector.tumblr.com/request=logger-v1` +1. A pod is created. It has annotation `injector.tumblr.com/request=logger:v1` 2. K8s webhooks out to this service, asking whether to allow this pod creation, and how to mutate it 3. If the pod is annotated with `injector.tumblr.com/status=injected`: Do nothing! Return "allowed" to pod creation -4. Pull the "logger-v1" sidecar config, patch the resource, and return it to k8s +4. Pull the "logger:v1" sidecar config, patch the resource, and return it to k8s 5. Pod will launch in k8s with the modified configuration A crappy ASCII diagram will help :) diff --git a/docs/sidecar-configuration-format.md b/docs/sidecar-configuration-format.md index 82ea283..17a1e00 100644 --- a/docs/sidecar-configuration-format.md +++ b/docs/sidecar-configuration-format.md @@ -12,10 +12,32 @@ A sidecar configuration looks like: # annotation, like: # "injector.tumblr.com/request=tumblr-php" # the "name: tumblr-php" must match a configuration below; + +# "name" identifies this sidecar uniquely to the injector. NOTE: it is an error to load +# 2 configuration with the same name! You may include version information in the name to disambiguate +# between newer versions of the same sidecar. For example: +# name: my-sidecar:v1.2 +# indicates "my-sidecar" is version "1.2". A request for `injector.tumblr.com/request: my-sidecar:v1.2` +# will return this configuration. If the version information is omitted, "latest" is assumed. +# `name: "test"` implies `name: test:latest`. +# * `injector.tumblr.com/request: my-sidecar` => `my-sidecar:latest` +# * `injector.tumblr.com/request: my-sidecar:latest` => `my-sidecar:latest` +# * `injector.tumblr.com/request: my-sidecar:v1.2` => `my-sidecar:v1.2` +name: "test:v1.2" + # Each InjectionConfig is a struct that adheres to kubernetes' volume and containers # spec. Any volumes injected are scoped to the namespace that the # resource exists within -name: "test" + +# Optionally, you can inherit from another sidecar configuration. This is useful to reduce +# duplication in your sidecars. Fields that appear in this config will override and replace +# fields in the inherited sidecar. We intelligently merge list fields as well, so top level +# keys are not blindly replaced, but merged instead. +# `inherits` is a file on disk to load the parent config from. +# NOTE: `inherits` is not supported when loading InjectionConfigs from ConfigMap +# NOTE: this is relative to the current file, and does not allow for absolute pathing! +inherits: "some-sidecar.yaml" + containers: # we inject a nginx container - name: sidecar-nginx diff --git a/go.mod b/go.mod index 150a36b..f56b51b 100644 --- a/go.mod +++ b/go.mod @@ -27,7 +27,7 @@ require ( github.com/prometheus/procfs v0.0.0-20181005140218-185b4288413d // indirect github.com/spf13/pflag v1.0.3 // indirect github.com/stretchr/testify v1.4.0 // indirect - golang.org/x/lint v0.0.0-20190930215403-16217165b5de // indirect + golang.org/x/lint v0.0.0-20191125180803-fdd1cda4f05f // indirect golang.org/x/oauth2 v0.0.0-20181106182150-f42d05182288 // indirect golang.org/x/time v0.0.0-20181108054448-85acf8d2951c // indirect google.golang.org/appengine v1.3.0 // indirect diff --git a/go.sum b/go.sum index df3736f..06a8255 100644 --- a/go.sum +++ b/go.sum @@ -62,13 +62,18 @@ golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2 h1:VklqNMn3ovrHsnt90Pveol golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w= golang.org/x/lint v0.0.0-20190930215403-16217165b5de h1:5hukYrvBGR8/eNkX5mdUezrA6JiaEZDtJb9Ei+1LlBs= golang.org/x/lint v0.0.0-20190930215403-16217165b5de/go.mod h1:6SW0HCj/g11FgYtHlgUYUwCkIfeOF89ocIRzGO/8vkc= +golang.org/x/lint v0.0.0-20191125180803-fdd1cda4f05f h1:J5lckAjkw6qYlOZNj90mLYNTEKDvWeuc1yieZ8qUzUE= +golang.org/x/lint v0.0.0-20191125180803-fdd1cda4f05f/go.mod h1:5qLYkcX4OjUUV8bRuDixDT3tpyyb+LUpUlRWLxfhWrs= golang.org/x/net v0.0.0-20180724234803-3673e40ba225/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4= golang.org/x/net v0.0.0-20181108082009-03003ca0c849 h1:FSqE2GGG7wzsYUsWiQ8MZrvEd1EOyU3NCF0AW3Wtltg= golang.org/x/net v0.0.0-20181108082009-03003ca0c849/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4= golang.org/x/net v0.0.0-20190311183353-d8887717615a h1:oWX7TPOiFAMXLq8o0ikBYfCJVlRHBcsciT5bXOrH628= golang.org/x/net v0.0.0-20190311183353-d8887717615a/go.mod h1:t9HGtf8HONx5eT2rtn7q6eTqICYqUVnKs3thJo3Qplg= +golang.org/x/net v0.0.0-20190620200207-3b0461eec859 h1:R/3boaszxrf1GEUWTVDzSKVwLmSJpwZ1yqXm8j0v2QI= +golang.org/x/net v0.0.0-20190620200207-3b0461eec859/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s= golang.org/x/oauth2 v0.0.0-20181106182150-f42d05182288 h1:JIqe8uIcRBHXDQVvZtHwp80ai3Lw3IJAeJEs55Dc1W0= golang.org/x/oauth2 v0.0.0-20181106182150-f42d05182288/go.mod h1:N/0e6XlmueqKjAGxoOufVs8QHGRruUQn6yWY3a++T0U= +golang.org/x/sync v0.0.0-20190423024810-112230192c58/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sys v0.0.0-20181107165924-66b7b1311ac8 h1:YoY1wS6JYVRpIfFngRf2HHo9R9dAne3xbkGOQ5rJXjU= golang.org/x/sys v0.0.0-20181107165924-66b7b1311ac8/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a h1:1BGLXjeY4akVXGgbC9HugT3Jv3hCI0z56oJR5vAMgBU= @@ -79,8 +84,12 @@ golang.org/x/time v0.0.0-20181108054448-85acf8d2951c h1:fqgJT0MGcGpPgpWU7VRdRjuA golang.org/x/time v0.0.0-20181108054448-85acf8d2951c/go.mod h1:tRJNPiyCQ0inRvYxbN9jk5I+vvW/OXSQhTDSoE431IQ= golang.org/x/tools v0.0.0-20190311212946-11955173bddd h1:/e+gpKk9r3dJobndpTytxS2gOy6m5uvpg+ISQoEcusQ= golang.org/x/tools v0.0.0-20190311212946-11955173bddd/go.mod h1:LCzVGOaR6xXOjkQ3onu1FJEFr0SW1gC7cKk1uF8kGRs= +golang.org/x/tools v0.0.0-20191125144606-a911d9008d1f h1:kDxGY2VmgABOe55qheT/TFqUMtcTHnomIPS1iv3G4Ms= +golang.org/x/tools v0.0.0-20191125144606-a911d9008d1f/go.mod h1:b+2E5dAYhXwXZwtnZ6UAqBI28+e2cm9otk0dWdXHAEo= +golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= google.golang.org/appengine v1.3.0 h1:FBSsiFRMz3LBeXIomRnVzrQwSDj4ibvcRexLG0LZGQk= google.golang.org/appengine v1.3.0/go.mod h1:xpcJRLb0r/rnEns0DIKYYv+WjYCduHsrkT7/EB5XEv4= +gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= gopkg.in/inf.v0 v0.9.1 h1:73M5CoZyi3ZLMOyDlQh031Cx6N9NDJ2Vvfl76EDAgDc= gopkg.in/inf.v0 v0.9.1/go.mod h1:cWUDdTG/fYaXco+Dcufb5Vnc6Gp2YChqWtbxRZE0mXw= diff --git a/internal/pkg/config/config.go b/internal/pkg/config/config.go index baabc4e..63bf2be 100644 --- a/internal/pkg/config/config.go +++ b/internal/pkg/config/config.go @@ -1,7 +1,6 @@ package config import ( - "crypto/sha256" "fmt" "io" "io/ioutil" @@ -17,6 +16,7 @@ import ( const ( annotationNamespaceDefault = "injector.tumblr.com" + defaultVersion = "latest" ) var ( @@ -24,11 +24,16 @@ var ( ErrMissingName = fmt.Errorf(`name field is required for an injection config`) // ErrNoConfigurationLoaded .. ErrNoConfigurationLoaded = fmt.Errorf(`at least one config must be present in the --config-directory`) + // ErrCannotMergeNilInjectionConfig indicates an error trying to merge `nil` into an InjectionConfig + ErrCannotMergeNilInjectionConfig = fmt.Errorf("cannot merge nil InjectionConfig") + // ErrUnsupportedNameVersionFormat indicates the format of the name is invalid + ErrUnsupportedNameVersionFormat = fmt.Errorf(`not a valid name or name:version format`) ) // InjectionConfig is a specific instance of a injected config, for a given annotation type InjectionConfig struct { Name string `json:"name"` + Inherits string `json:"inherits"` Containers []corev1.Container `json:"containers"` Volumes []corev1.Volume `json:"volumes"` Environment []corev1.EnvVar `json:"env"` @@ -36,6 +41,8 @@ type InjectionConfig struct { HostAliases []corev1.HostAlias `json:"hostAliases"` InitContainers []corev1.Container `json:"initContainers"` ServiceAccountName string `json:"serviceAccountName"` + + version string } // Config is a struct indicating how a given injection should be configured @@ -47,7 +54,37 @@ type Config struct { // String returns a string representation of the config func (c *InjectionConfig) String() string { - return fmt.Sprintf("%s: %d containers, %d init containers, %d volumes, %d environment vars, %d volume mounts, %d host aliases", c.Name, len(c.Containers), len(c.InitContainers), len(c.Volumes), len(c.Environment), len(c.VolumeMounts), len(c.HostAliases)) + inheritsString := "" + if c.Inherits != "" { + inheritsString = fmt.Sprintf(" (inherits %s)", c.Inherits) + } + + return fmt.Sprintf("%s%s: %d containers, %d init containers, %d volumes, %d environment vars, %d volume mounts, %d host aliases", + c.FullName(), + inheritsString, + len(c.Containers), + len(c.InitContainers), + len(c.Volumes), + len(c.Environment), + len(c.VolumeMounts), + len(c.HostAliases)) +} + +// Version returns the parsed version of this injection config. If no version is specified, +// "latest" is returned. The version is extracted from the request annotation, i.e. +// injector.tumblr.com/request: my-sidecar:1.2, where "1.2" is the version. +func (c *InjectionConfig) Version() string { + if c.version == "" { + return defaultVersion + } + + return c.version +} + +// FullName returns the full identifier of this sidecar - both the Name, and the Version(), formatted like +// "${.Name}:${.Version}" +func (c *InjectionConfig) FullName() string { + return canonicalizeConfigName(c.Name, c.Version()) } // ReplaceInjectionConfigs will take a list of new InjectionConfigs, and replace the current configuration with them. @@ -56,8 +93,9 @@ func (c *Config) ReplaceInjectionConfigs(replacementConfigs []*InjectionConfig) c.Lock() defer c.Unlock() c.Injections = map[string]*InjectionConfig{} + for _, r := range replacementConfigs { - c.Injections[r.Name] = r + c.Injections[r.FullName()] = r } } @@ -66,7 +104,15 @@ func (c *Config) ReplaceInjectionConfigs(replacementConfigs []*InjectionConfig) func (c *Config) HasInjectionConfig(key string) bool { c.RLock() defer c.RUnlock() - _, ok := c.Injections[strings.ToLower(key)] + + name, version, err := configNameFields(key) + if err != nil { + return false + } + fullKey := canonicalizeConfigName(name, version) + + _, ok := c.Injections[fullKey] + return ok } @@ -74,11 +120,18 @@ func (c *Config) HasInjectionConfig(key string) bool { func (c *Config) GetInjectionConfig(key string) (*InjectionConfig, error) { c.RLock() defer c.RUnlock() - k := strings.ToLower(key) - i, ok := c.Injections[k] + + name, version, err := configNameFields(key) + if err != nil { + return nil, err + } + fullKey := canonicalizeConfigName(name, version) + + i, ok := c.Injections[fullKey] if !ok { - return nil, fmt.Errorf("no injection config found for annotation %s", key) + return nil, fmt.Errorf("no injection config found for annotation %s", fullKey) } + return i, nil } @@ -89,16 +142,19 @@ func LoadConfigDirectory(path string) (*Config, error) { } glob := filepath.Join(path, "*.yaml") matches, err := filepath.Glob(glob) + if err != nil { return nil, err } + for _, p := range matches { c, err := LoadInjectionConfigFromFilePath(p) if err != nil { glog.Errorf("Error reading injection config from %s: %v", p, err) return nil, err } - cfg.Injections[c.Name] = c + + cfg.Injections[c.FullName()] = c } if len(cfg.Injections) == 0 { @@ -114,15 +170,146 @@ func LoadConfigDirectory(path string) (*Config, error) { return &cfg, nil } +// Merge mutates base by merging in fields from child, to create an inheritance +// functionality. +func (base *InjectionConfig) Merge(child *InjectionConfig) error { + if child == nil { + return ErrCannotMergeNilInjectionConfig + } + // for all fields, merge child into base, eventually returning base + base.Name = child.Name + base.version = child.version + base.Inherits = child.Inherits + + // merge containers + for _, cctr := range child.Containers { + contains := false + + for bi, bctr := range base.Containers { + if bctr.Name == cctr.Name { + contains = true + base.Containers[bi] = cctr + } + } + + if !contains { + base.Containers = append(base.Containers, cctr) + } + } + + // merge volumes + for _, cv := range child.Volumes { + contains := false + + for bi, bv := range base.Volumes { + if bv.Name == cv.Name { + contains = true + base.Volumes[bi] = cv + } + } + + if !contains { + base.Volumes = append(base.Volumes, cv) + } + } + + // merge environment + for _, cv := range child.Environment { + contains := false + + for bi, bv := range base.Environment { + if bv.Name == cv.Name { + contains = true + base.Environment[bi] = cv + } + } + + if !contains { + base.Environment = append(base.Environment, cv) + } + } + + // merge volume mounts + for _, cv := range child.VolumeMounts { + contains := false + + for bi, bv := range base.VolumeMounts { + if bv.Name == cv.Name { + contains = true + base.VolumeMounts[bi] = cv + } + } + + if !contains { + base.VolumeMounts = append(base.VolumeMounts, cv) + } + } + + // merge host aliases + // note: we do not need to merge things, as entries are not keyed + base.HostAliases = append(base.HostAliases, child.HostAliases...) + + // merge init containers + for _, cv := range child.InitContainers { + contains := false + + for bi, bv := range base.InitContainers { + if bv.Name == cv.Name { + contains = true + base.InitContainers[bi] = cv + } + } + + if !contains { + base.InitContainers = append(base.InitContainers, cv) + } + } + + return nil +} + // LoadInjectionConfigFromFilePath returns a InjectionConfig given a yaml file on disk +// NOTE: if the InjectionConfig loaded has an Inherits field, we recursively load from Inherits +// and merge the InjectionConfigs to create an inheritance pattern. Inherits is not supported for +// configs loaded via `LoadInjectionConfig` func LoadInjectionConfigFromFilePath(configFile string) (*InjectionConfig, error) { f, err := os.Open(configFile) - defer f.Close() if err != nil { return nil, fmt.Errorf("error loading injection config from file %s: %s", configFile, err.Error()) } + + defer f.Close() glog.V(3).Infof("Loading injection config from file %s", configFile) - return LoadInjectionConfig(f) + + ic, err := LoadInjectionConfig(f) + if err != nil { + return nil, err + } + + // Support inheritance from an InjectionConfig loaded from a file on disk + if ic.Inherits != "" { + // all Inherits are relative to the directory the current file is in, and are cleaned + // prior to use. + basedir := filepath.Dir(filepath.Clean(f.Name())) + cleanPath := filepath.Join(basedir, ic.Inherits) + glog.V(4).Infof("%s inherits from %s", ic.FullName(), ic.Inherits) + + base, err := LoadInjectionConfigFromFilePath(cleanPath) + if err != nil { + return nil, err + } + + err = base.Merge(ic) + if err != nil { + return nil, err + } + + ic = base + } + + glog.V(3).Infof("Loaded injection config %s version=%s", ic.Name, ic.Version()) + + return ic, nil } // LoadInjectionConfig takes an io.Reader and parses out an injectionconfig @@ -141,7 +328,34 @@ func LoadInjectionConfig(reader io.Reader) (*InjectionConfig, error) { return nil, ErrMissingName } - glog.V(3).Infof("Loaded injection config %s sha256sum=%x", cfg.Name, sha256.Sum256(data)) + // we need to split the Name field apart into a Name and Version component + cfg.Name, cfg.version, err = configNameFields(cfg.Name) + if err != nil { + return nil, err + } return &cfg, nil } + +// given a name of a config, extract the name and version. Format is "name[:version]" where :version +// is optional, and is assumed to be "latest" if omitted. +func configNameFields(shortName string) (name, version string, err error) { + substrings := strings.Split(shortName, ":") + + switch len(substrings) { + case 1: + return substrings[0], defaultVersion, nil + case 2: + if substrings[1] == "" { + return substrings[0], defaultVersion, nil + } + + return substrings[0], substrings[1], nil + default: + return "", "", ErrUnsupportedNameVersionFormat + } +} + +func canonicalizeConfigName(name, version string) string { + return strings.ToLower(fmt.Sprintf("%s:%s", name, version)) +} diff --git a/internal/pkg/config/config_test.go b/internal/pkg/config/config_test.go index 7e6ec9f..a5f030c 100644 --- a/internal/pkg/config/config_test.go +++ b/internal/pkg/config/config_test.go @@ -1,9 +1,10 @@ package config import ( + "fmt" "testing" - _ "github.com/tumblr/k8s-sidecar-injector/internal/pkg/testing" + testhelper "github.com/tumblr/k8s-sidecar-injector/internal/pkg/testing" ) // config struct for testing: where to find the file and what we expect to find in it @@ -23,127 +24,201 @@ var ( // location of the fixture sidecar files fixtureSidecarsDir = "test/fixtures/sidecars" + testBadConfigs = map[string]testhelper.ConfigExpectation{ + // test that a name with spurious use of ":" errors out on load + "versioned:with:extra:data:v3": testhelper.ConfigExpectation{ + Path: fixtureSidecarsDir + "/bad/init-containers-colons-v3.yaml", + LoadError: ErrUnsupportedNameVersionFormat, + }, + "missing name": testhelper.ConfigExpectation{ + Path: fixtureSidecarsDir + "/bad/missing-name.yaml", + LoadError: ErrMissingName, + }, + "inheritance filenotfound": testhelper.ConfigExpectation{ + Path: fixtureSidecarsDir + "/bad/inheritance-filenotfound.yaml", + LoadError: fmt.Errorf(`error loading injection config from file test/fixtures/sidecars/bad/some-missing-file.yaml: open test/fixtures/sidecars/bad/some-missing-file.yaml: no such file or directory`), + }, + "inheritance escape": testhelper.ConfigExpectation{ + Path: fixtureSidecarsDir + "/bad/inheritance-escape.yaml", + LoadError: fmt.Errorf(`error loading injection config from file test/fixtures/etc/passwd: open test/fixtures/etc/passwd: no such file or directory`), + }, + } + // test files and expectations - testConfigs = map[string]configExpectation{ - "sidecar-test": configExpectation{ - name: "sidecar-test", - path: fixtureSidecarsDir + "/sidecar-test.yaml", - expectedEnvVarCount: 2, - expectedContainerCount: 2, - expectedVolumeCount: 1, - expectedVolumeMountCount: 0, - expectedHostAliasCount: 0, - expectedInitContainerCount: 0, - expectedServiceAccount: "", + testGoodConfigs = map[string]testhelper.ConfigExpectation{ + "sidecar-test": testhelper.ConfigExpectation{ + Name: "sidecar-test", + Version: "latest", + Path: fixtureSidecarsDir + "/sidecar-test.yaml", + EnvCount: 2, + ContainerCount: 2, + VolumeCount: 1, + VolumeMountCount: 0, + HostAliasCount: 0, + InitContainerCount: 0, + }, + "complex-sidecar": testhelper.ConfigExpectation{ + Name: "complex-sidecar", + Version: "v420.69", + Path: fixtureSidecarsDir + "/complex-sidecar.yaml", + EnvCount: 0, + ContainerCount: 4, + VolumeCount: 1, + VolumeMountCount: 0, + HostAliasCount: 0, + InitContainerCount: 0, + }, + "env1": testhelper.ConfigExpectation{ + Name: "env1", + Version: "latest", + Path: fixtureSidecarsDir + "/env1.yaml", + EnvCount: 3, + ContainerCount: 0, + VolumeCount: 0, + VolumeMountCount: 0, + HostAliasCount: 0, + InitContainerCount: 0, }, - "complex-sidecar": configExpectation{ - name: "complex-sidecar", - path: fixtureSidecarsDir + "/complex-sidecar.yaml", - expectedEnvVarCount: 0, - expectedContainerCount: 4, - expectedVolumeCount: 1, - expectedVolumeMountCount: 0, - expectedHostAliasCount: 0, - expectedInitContainerCount: 0, - expectedServiceAccount: "", + "volume-mounts": testhelper.ConfigExpectation{ + Name: "volume-mounts", + Version: "latest", + Path: fixtureSidecarsDir + "/volume-mounts.yaml", + EnvCount: 2, + ContainerCount: 3, + VolumeCount: 2, + VolumeMountCount: 1, + HostAliasCount: 0, + InitContainerCount: 0, }, - "env1": configExpectation{ - name: "env1", - path: fixtureSidecarsDir + "/env1.yaml", - expectedEnvVarCount: 3, - expectedContainerCount: 0, - expectedVolumeCount: 0, - expectedVolumeMountCount: 0, - expectedHostAliasCount: 0, - expectedInitContainerCount: 0, - expectedServiceAccount: "", + "host-aliases": testhelper.ConfigExpectation{ + Name: "host-aliases", + Version: "latest", + Path: fixtureSidecarsDir + "/host-aliases.yaml", + EnvCount: 2, + ContainerCount: 1, + VolumeCount: 0, + VolumeMountCount: 0, + HostAliasCount: 6, + InitContainerCount: 0, }, - "volume-mounts": configExpectation{ - name: "volume-mounts", - path: fixtureSidecarsDir + "/volume-mounts.yaml", - expectedEnvVarCount: 2, - expectedContainerCount: 3, - expectedVolumeCount: 2, - expectedVolumeMountCount: 1, - expectedHostAliasCount: 0, - expectedInitContainerCount: 0, - expectedServiceAccount: "", + "init-containers": testhelper.ConfigExpectation{ + Name: "init-containers", + Version: "latest", + Path: fixtureSidecarsDir + "/init-containers.yaml", + EnvCount: 0, + ContainerCount: 2, + VolumeCount: 0, + VolumeMountCount: 0, + HostAliasCount: 0, + InitContainerCount: 1, }, - "host-aliases": configExpectation{ - name: "host-aliases", - path: fixtureSidecarsDir + "/host-aliases.yaml", - expectedEnvVarCount: 2, - expectedContainerCount: 1, - expectedVolumeCount: 0, - expectedVolumeMountCount: 0, - expectedHostAliasCount: 6, - expectedInitContainerCount: 0, - expectedServiceAccount: "", + "versioned1": testhelper.ConfigExpectation{ + Name: "init-containers", + Version: "v2", + Path: fixtureSidecarsDir + "/init-containers-v2.yaml", + EnvCount: 0, + ContainerCount: 2, + VolumeCount: 0, + VolumeMountCount: 0, + HostAliasCount: 0, + InitContainerCount: 1, }, - "init-containers": configExpectation{ - name: "init-containers", - path: fixtureSidecarsDir + "/init-containers.yaml", - expectedEnvVarCount: 0, - expectedContainerCount: 2, - expectedVolumeCount: 0, - expectedVolumeMountCount: 0, - expectedHostAliasCount: 0, - expectedInitContainerCount: 1, - expectedServiceAccount: "", + // test simple inheritance + "simple inheritance from complex-sidecar": testhelper.ConfigExpectation{ + Name: "inheritance-complex", + Version: "v1", + Path: fixtureSidecarsDir + "/inheritance-1.yaml", + EnvCount: 2, + ContainerCount: 5, + VolumeCount: 2, + VolumeMountCount: 0, + HostAliasCount: 1, + InitContainerCount: 1, }, - "service-account": configExpectation{ - name: "service-account", - path: fixtureSidecarsDir + "/service-account.yaml", - expectedEnvVarCount: 0, - expectedContainerCount: 0, - expectedVolumeCount: 0, - expectedVolumeMountCount: 0, - expectedHostAliasCount: 0, - expectedInitContainerCount: 0, - expectedServiceAccount: "fuck", + // test deep inheritance + "deep inheritance from inheritance-complex": testhelper.ConfigExpectation{ + Name: "inheritance-deep", + Version: "v2", + Path: fixtureSidecarsDir + "/inheritance-deep-2.yaml", + EnvCount: 3, + ContainerCount: 6, + VolumeCount: 3, + VolumeMountCount: 0, + HostAliasCount: 3, + InitContainerCount: 2, + }, + "service-account": testhelper.ConfigExpectation{ + Name: "service-account", + Version: "latest", + Path: fixtureSidecarsDir + "/service-account.yaml", + EnvCount: 0, + ContainerCount: 0, + VolumeCount: 0, + VolumeMountCount: 0, + HostAliasCount: 0, + InitContainerCount: 0, + ServiceAccount: "fuck", }, } ) -// TestConfigs: load configs from filepath and check if we load what we expected -func TestConfigs(t *testing.T) { - for _, testConfig := range testConfigs { - c, err := LoadInjectionConfigFromFilePath(testConfig.path) - if err != nil { - t.Error(err) - t.Fail() +func TestConfigsLoadErrors(t *testing.T) { + for _, testConfig := range testBadConfigs { + _, err := LoadInjectionConfigFromFilePath(testConfig.Path) + if err == nil || testConfig.LoadError == nil { + t.Fatal("error was nil or LoadError was nil - this test should only be testing load errors") + } + if testConfig.LoadError.Error() != err.Error() { + t.Fatalf("expected %s load to produce error %v but got %v", testConfig.Path, testConfig.LoadError, err) + } + } +} + +// TestGoodConfigs: load configs from filepath and check if we load what we expected +func TestGoodConfigs(t *testing.T) { + for _, testConfig := range testGoodConfigs { + c, err := LoadInjectionConfigFromFilePath(testConfig.Path) + if testConfig.LoadError != err { + t.Fatalf("expected %s load to produce error %v but got %v", testConfig.Path, testConfig.LoadError, err) + } + if testConfig.LoadError != nil { + // if we expected a load error, and we made it here, continue, because we do not need to test + // anything about the loaded InjectionConfig + continue } - if c.Name != testConfig.name { - t.Errorf("expected %s Name loaded from %s but got %s", testConfig.name, testConfig.path, c.Name) - t.Fail() + if c.Name != testConfig.Name { + t.Fatalf("expected %s Name loaded from %s but got %s", testConfig.Name, testConfig.Path, c.Name) } - if len(c.Environment) != testConfig.expectedEnvVarCount { - t.Errorf("expected %d EnvVars loaded from %s but got %d", testConfig.expectedEnvVarCount, testConfig.path, len(c.Environment)) - t.Fail() + if c.Version() != testConfig.Version { + t.Fatalf("expected %s Version() loaded from %s but got %s", testConfig.Version, testConfig.Path, c.Version()) } - if len(c.Containers) != testConfig.expectedContainerCount { - t.Errorf("expected %d Containers loaded from %s but got %d", testConfig.expectedContainerCount, testConfig.path, len(c.Containers)) - t.Fail() + if c.FullName() != testConfig.FullName() { + t.Fatalf("expected FullName() %s loaded from %s but got %s", testConfig.FullName(), testConfig.Path, c.FullName()) } - if len(c.Volumes) != testConfig.expectedVolumeCount { - t.Errorf("expected %d Volumes loaded from %s but got %d", testConfig.expectedVolumeCount, testConfig.path, len(c.Volumes)) - t.Fail() + if len(c.Environment) != testConfig.EnvCount { + t.Fatalf("expected %d Envs loaded from %s but got %d", testConfig.EnvCount, testConfig.Path, len(c.Environment)) } - if len(c.VolumeMounts) != testConfig.expectedVolumeMountCount { - t.Fatalf("expected %d VolumeMounts loaded from %s but got %d", testConfig.expectedVolumeMountCount, testConfig.path, len(c.VolumeMounts)) + if len(c.Containers) != testConfig.ContainerCount { + t.Fatalf("expected %d Containers loaded from %s but got %d", testConfig.ContainerCount, testConfig.Path, len(c.Containers)) } - if len(c.HostAliases) != testConfig.expectedHostAliasCount { - t.Fatalf("expected %d HostAliases loaded from %s but got %d", testConfig.expectedHostAliasCount, testConfig.path, len(c.HostAliases)) + if len(c.Volumes) != testConfig.VolumeCount { + t.Fatalf("expected %d Volumes loaded from %s but got %d", testConfig.VolumeCount, testConfig.Path, len(c.Volumes)) } - if len(c.InitContainers) != testConfig.expectedInitContainerCount { - t.Fatalf("expected %d InitContainers loaded from %s but got %d", testConfig.expectedInitContainerCount, testConfig.path, len(c.InitContainers)) + if len(c.VolumeMounts) != testConfig.VolumeMountCount { + t.Fatalf("expected %d VolumeMounts loaded from %s but got %d", testConfig.VolumeMountCount, testConfig.Path, len(c.VolumeMounts)) + } + if len(c.HostAliases) != testConfig.HostAliasCount { + t.Fatalf("expected %d HostAliases loaded from %s but got %d", testConfig.HostAliasCount, testConfig.Path, len(c.HostAliases)) + } + if len(c.InitContainers) != testConfig.InitContainerCount { + t.Fatalf("expected %d InitContainers loaded from %s but got %d", testConfig.InitContainerCount, testConfig.Path, len(c.InitContainers)) } } } // TestLoadConfig: Check if we get all the configs func TestLoadConfig(t *testing.T) { - expectedNumInjectionsConfig := len(testConfigs) + expectedNumInjectionsConfig := len(testGoodConfigs) c, err := LoadConfigDirectory(fixtureSidecarsDir) if err != nil { t.Fatal(err) @@ -158,33 +233,42 @@ func TestLoadConfig(t *testing.T) { // TestFetInjectionConfig: Check if we can properly load a config by name and see if we read the correct values from it func TestGetInjectionConfig(t *testing.T) { - cfg := testConfigs["sidecar-test"] + cfg := testGoodConfigs["sidecar-test"] c, err := LoadConfigDirectory(fixtureSidecarsDir) if err != nil { t.Fatal(err) } - i, err := c.GetInjectionConfig(cfg.name) + i, err := c.GetInjectionConfig(cfg.FullName()) if err != nil { t.Fatal(err) } - if len(i.Environment) != cfg.expectedEnvVarCount { - t.Fatalf("expected %d envvars, but got %d", cfg.expectedEnvVarCount, len(i.Environment)) + if i.Name != cfg.Name { + t.Fatalf("expected name %s, but got %s", cfg.Name, i.Name) + } + if i.Version() != cfg.Version { + t.Fatalf("expected version %s, but got %s", cfg.Version, i.Version()) + } + if i.FullName() != cfg.FullName() { + t.Fatalf("expected FullName %s, but got %s", cfg.FullName(), i.FullName()) + } + if len(i.Environment) != cfg.EnvCount { + t.Fatalf("expected %d Envs, but got %d", cfg.EnvCount, len(i.Environment)) } - if len(i.Containers) != cfg.expectedContainerCount { - t.Fatalf("expected %d container, but got %d", cfg.expectedContainerCount, len(i.Containers)) + if len(i.Containers) != cfg.ContainerCount { + t.Fatalf("expected %d container, but got %d", cfg.ContainerCount, len(i.Containers)) } - if len(i.Volumes) != cfg.expectedVolumeCount { - t.Fatalf("expected %d volume, but got %d", cfg.expectedVolumeCount, len(i.Volumes)) + if len(i.Volumes) != cfg.VolumeCount { + t.Fatalf("expected %d volume, but got %d", cfg.VolumeCount, len(i.Volumes)) } - if len(i.VolumeMounts) != cfg.expectedVolumeMountCount { - t.Fatalf("expected %d VolumeMounts, but got %d", cfg.expectedVolumeMountCount, len(i.VolumeMounts)) + if len(i.VolumeMounts) != cfg.VolumeMountCount { + t.Fatalf("expected %d VolumeMounts, but got %d", cfg.VolumeMountCount, len(i.VolumeMounts)) } - if len(i.HostAliases) != cfg.expectedHostAliasCount { - t.Fatalf("expected %d HostAliases, but got %d", cfg.expectedHostAliasCount, len(i.HostAliases)) + if len(i.HostAliases) != cfg.HostAliasCount { + t.Fatalf("expected %d HostAliases, but got %d", cfg.HostAliasCount, len(i.HostAliases)) } - if len(i.InitContainers) != cfg.expectedInitContainerCount { - t.Fatalf("expected %d InitContainers, but got %d", cfg.expectedInitContainerCount, len(i.InitContainers)) + if len(i.InitContainers) != cfg.InitContainerCount { + t.Fatalf("expected %d InitContainers, but got %d", cfg.InitContainerCount, len(i.InitContainers)) } } diff --git a/internal/pkg/config/watcher/loader_test.go b/internal/pkg/config/watcher/loader_test.go index 7c955fd..28df237 100644 --- a/internal/pkg/config/watcher/loader_test.go +++ b/internal/pkg/config/watcher/loader_test.go @@ -9,119 +9,124 @@ import ( "testing" "github.com/tumblr/k8s-sidecar-injector/internal/pkg/config" - _ "github.com/tumblr/k8s-sidecar-injector/internal/pkg/testing" + testhelper "github.com/tumblr/k8s-sidecar-injector/internal/pkg/testing" "gopkg.in/yaml.v2" v1 "k8s.io/api/core/v1" ) -type injectionConfigExpectation struct { - name string - volumeCount int - envCount int - containerCount int - volumeMountCount int - hostAliasCount int - initContainerCount int -} - var ( - // maps a k8s ConfigMap fixture in test/fixtures/k8s/ => InjectionConfigExpectation - ExpectedInjectionConfigFixtures = map[string][]injectionConfigExpectation{ - "configmap-env1": []injectionConfigExpectation{ - injectionConfigExpectation{ - name: "env1", - volumeCount: 0, - envCount: 3, - containerCount: 0, - volumeMountCount: 0, - hostAliasCount: 0, - initContainerCount: 0, + fixtureSidecarsDir = "test/fixtures/sidecars" + fixtureK8sDir = "test/fixtures/k8s" + + // maps a k8s ConfigMap fixture in test/fixtures/k8s/ => testhelper.ConfigExpectation + ExpectedInjectionConfigFixtures = map[string][]testhelper.ConfigExpectation{ + "configmap-env1": []testhelper.ConfigExpectation{ + testhelper.ConfigExpectation{ + Name: "env1", + Version: "latest", + Path: fixtureSidecarsDir + "/env1.yaml", + VolumeCount: 0, + EnvCount: 3, + ContainerCount: 0, + VolumeMountCount: 0, + HostAliasCount: 0, + InitContainerCount: 0, }, }, - "configmap-sidecar-test": []injectionConfigExpectation{ - injectionConfigExpectation{ - name: "sidecar-test", - volumeCount: 1, - envCount: 2, - containerCount: 2, - volumeMountCount: 0, - hostAliasCount: 0, - initContainerCount: 0, + "configmap-sidecar-test": []testhelper.ConfigExpectation{ + testhelper.ConfigExpectation{ + Name: "sidecar-test", + Version: "latest", + Path: fixtureSidecarsDir + "/sidecar-test.yaml", + VolumeCount: 1, + EnvCount: 2, + ContainerCount: 2, + VolumeMountCount: 0, + HostAliasCount: 0, + InitContainerCount: 0, }, }, - "configmap-complex-sidecar": []injectionConfigExpectation{ - injectionConfigExpectation{ - name: "complex-sidecar", - volumeCount: 1, - envCount: 0, - containerCount: 4, - volumeMountCount: 0, - hostAliasCount: 0, - initContainerCount: 0, + "configmap-complex-sidecar": []testhelper.ConfigExpectation{ + testhelper.ConfigExpectation{ + Name: "complex-sidecar", + Version: "v420.69", + Path: fixtureSidecarsDir + "/complex-sidecar.yaml", + VolumeCount: 1, + EnvCount: 0, + ContainerCount: 4, + VolumeMountCount: 0, + HostAliasCount: 0, + InitContainerCount: 0, }, }, - "configmap-multiple1": []injectionConfigExpectation{ - injectionConfigExpectation{ - name: "env1", - volumeCount: 0, - envCount: 3, - containerCount: 0, - volumeMountCount: 0, - hostAliasCount: 0, - initContainerCount: 0, + "configmap-multiple1": []testhelper.ConfigExpectation{ + testhelper.ConfigExpectation{ + Name: "env1", + Version: "latest", + Path: fixtureSidecarsDir + "/env1.yaml", + VolumeCount: 0, + EnvCount: 3, + ContainerCount: 0, + VolumeMountCount: 0, + HostAliasCount: 0, + InitContainerCount: 0, }, - injectionConfigExpectation{ - name: "sidecar-test", - volumeCount: 1, - envCount: 2, - containerCount: 2, - volumeMountCount: 0, - hostAliasCount: 0, - initContainerCount: 0, + testhelper.ConfigExpectation{ + Name: "sidecar-test", + Version: "latest", + Path: fixtureSidecarsDir + "/sidecar-test.yaml", + VolumeCount: 1, + EnvCount: 2, + ContainerCount: 2, + VolumeMountCount: 0, + HostAliasCount: 0, + InitContainerCount: 0, }, }, - "configmap-volume-mounts": []injectionConfigExpectation{ - injectionConfigExpectation{ - name: "volume-mounts", - volumeCount: 2, - envCount: 2, - containerCount: 3, - volumeMountCount: 1, - hostAliasCount: 0, - initContainerCount: 0, + "configmap-volume-mounts": []testhelper.ConfigExpectation{ + testhelper.ConfigExpectation{ + Name: "volume-mounts", + Version: "latest", + Path: fixtureSidecarsDir + "/volume-mounts.yaml", + VolumeCount: 2, + EnvCount: 2, + ContainerCount: 3, + VolumeMountCount: 1, + HostAliasCount: 0, + InitContainerCount: 0, }, }, - "configmap-host-aliases": []injectionConfigExpectation{ - injectionConfigExpectation{ - name: "host-aliases", - volumeCount: 0, - envCount: 2, - containerCount: 1, - volumeMountCount: 0, - hostAliasCount: 6, - initContainerCount: 0, + "configmap-host-aliases": []testhelper.ConfigExpectation{ + testhelper.ConfigExpectation{ + Name: "host-aliases", + Version: "latest", + Path: fixtureSidecarsDir + "/host-aliases.yaml", + VolumeCount: 0, + EnvCount: 2, + ContainerCount: 1, + VolumeMountCount: 0, + HostAliasCount: 6, + InitContainerCount: 0, }, }, - "configmap-init-containers": []injectionConfigExpectation{ - injectionConfigExpectation{ - name: "init-containers", - volumeCount: 0, - envCount: 0, - containerCount: 2, - volumeMountCount: 0, - hostAliasCount: 0, - initContainerCount: 1, + "configmap-init-containers": []testhelper.ConfigExpectation{ + testhelper.ConfigExpectation{ + Name: "init-containers", + Version: "latest", + Path: fixtureSidecarsDir + "/init-containers.yaml", + VolumeCount: 0, + EnvCount: 0, + ContainerCount: 2, + VolumeMountCount: 0, + HostAliasCount: 0, + InitContainerCount: 1, }, }, } ) func k8sFixture(f string) string { - return fmt.Sprintf("test/fixtures/k8s/%s.yaml", f) -} - -func injectionConfigFixture(e injectionConfigExpectation) string { - return fmt.Sprintf("test/fixtures/sidecars/%s.yaml", e.name) + return fmt.Sprintf("%s/%s.yaml", fixtureK8sDir, f) } func TestLoadFromConfigMap(t *testing.T) { @@ -152,12 +157,12 @@ func TestLoadFromConfigMap(t *testing.T) { // make sure all the appropriate names are present expectedNames := make([]string, len(expectedFixtures)) for i, f := range expectedFixtures { - expectedNames[i] = f.name + expectedNames[i] = f.FullName() } sort.Strings(expectedNames) actualNames := []string{} for _, x := range ics { - actualNames = append(actualNames, x.Name) + actualNames = append(actualNames, x.FullName()) } sort.Strings(actualNames) if strings.Join(expectedNames, ",") != strings.Join(actualNames, ",") { @@ -165,33 +170,42 @@ func TestLoadFromConfigMap(t *testing.T) { } for _, expectedICF := range expectedFixtures { - expectedicFile := injectionConfigFixture(expectedICF) + expectedicFile := expectedICF.Path ic, err := config.LoadInjectionConfigFromFilePath(expectedicFile) if err != nil { t.Fatalf("unable to load expected fixture %s: %s", expectedicFile, err.Error()) } - if len(ic.Environment) != expectedICF.envCount { - t.Fatalf("expected %d environment variables in %s, but found %d", expectedICF.envCount, expectedICF.name, len(ic.Environment)) + if ic.Name != expectedICF.Name { + t.Fatalf("expected %s Name in %s, but found %s", expectedICF.Name, expectedICF.Path, ic.Name) + } + if ic.Version() != expectedICF.Version { + t.Fatalf("expected %s Version in %s, but found %s", expectedICF.Version, expectedICF.Path, ic.Version()) + } + if ic.FullName() != expectedICF.FullName() { + t.Fatalf("expected %s FullName() in %s, but found %s", expectedICF.FullName(), expectedICF.Path, ic.FullName()) + } + if len(ic.Environment) != expectedICF.EnvCount { + t.Fatalf("expected %d environment variables in %s, but found %d", expectedICF.EnvCount, expectedICF.Path, len(ic.Environment)) } - if len(ic.Containers) != expectedICF.containerCount { - t.Fatalf("expected %d containers in %s, but found %d", expectedICF.containerCount, expectedICF.name, len(ic.Containers)) + if len(ic.Containers) != expectedICF.ContainerCount { + t.Fatalf("expected %d containers in %s, but found %d", expectedICF.ContainerCount, expectedICF.Path, len(ic.Containers)) } - if len(ic.Volumes) != expectedICF.volumeCount { - t.Fatalf("expected %d volumes in %s, but found %d", expectedICF.volumeCount, expectedICF.name, len(ic.Volumes)) + if len(ic.Volumes) != expectedICF.VolumeCount { + t.Fatalf("expected %d volumes in %s, but found %d", expectedICF.VolumeCount, expectedICF.Path, len(ic.Volumes)) } - if len(ic.VolumeMounts) != expectedICF.volumeMountCount { - t.Fatalf("expected %d volume mounts in %s, but found %d", expectedICF.volumeMountCount, expectedICF.name, len(ic.VolumeMounts)) + if len(ic.VolumeMounts) != expectedICF.VolumeMountCount { + t.Fatalf("expected %d volume mounts in %s, but found %d", expectedICF.VolumeMountCount, expectedICF.Path, len(ic.VolumeMounts)) } - if len(ic.HostAliases) != expectedICF.hostAliasCount { - t.Fatalf("expected %d host aliases in %s, but found %d", expectedICF.hostAliasCount, expectedICF.name, len(ic.HostAliases)) + if len(ic.HostAliases) != expectedICF.HostAliasCount { + t.Fatalf("expected %d host aliases in %s, but found %d", expectedICF.HostAliasCount, expectedICF.Path, len(ic.HostAliases)) } - if len(ic.InitContainers) != expectedICF.initContainerCount { - t.Fatalf("expected %d init containers in %s, but found %d", expectedICF.initContainerCount, expectedICF.name, len(ic.InitContainers)) + if len(ic.InitContainers) != expectedICF.InitContainerCount { + t.Fatalf("expected %d init containers in %s, but found %d", expectedICF.InitContainerCount, expectedICF.Path, len(ic.InitContainers)) } for _, actualIC := range ics { - if ic.Name == actualIC.Name { + if ic.FullName() == actualIC.FullName() { if ic.String() != actualIC.String() { - t.Fatalf("expected %s to equal %s", ic.String(), actualIC.String()) + t.Fatalf("%s: expected %s to equal %s", expectedICF.Path, ic.String(), actualIC.String()) } } } diff --git a/internal/pkg/testing/config.go b/internal/pkg/testing/config.go new file mode 100644 index 0000000..ffa7cc7 --- /dev/null +++ b/internal/pkg/testing/config.go @@ -0,0 +1,30 @@ +package testing + +import ( + "fmt" + "strings" +) + +// ConfigExpectation struct for testing: where to find the file and what we expect to find in it +type ConfigExpectation struct { + // name is not the Name in the loaded config, but only the "some-config" of "some-config:1.2" + Name string + // version is the parsed version string, or "latest" if omitted + Version string + // Path is the path to the YAML to load the sidecar yaml from + Path string + EnvCount int + ContainerCount int + VolumeCount int + VolumeMountCount int + HostAliasCount int + InitContainerCount int + ServiceAccount string + + // LoadError is an error, if any, that is expected during load + LoadError error +} + +func (x *ConfigExpectation) FullName() string { + return strings.ToLower(fmt.Sprintf("%s:%s", x.Name, x.Version)) +} diff --git a/pkg/server/webhook.go b/pkg/server/webhook.go index 3a12e35..2f32967 100644 --- a/pkg/server/webhook.go +++ b/pkg/server/webhook.go @@ -133,7 +133,8 @@ func (whsvr *WebhookServer) requestAnnotationKey() string { return whsvr.Config.AnnotationNamespace + "/request" } -// Check whether the target resoured need to be mutated +// Check whether the target resoured need to be mutated. returns the canonicalized full name of the injection config +// if found, or an error if not. func (whsvr *WebhookServer) getSidecarConfigurationRequested(ignoredList []string, metadata *metav1.ObjectMeta) (string, error) { // skip special kubernetes system namespaces for _, namespace := range ignoredList { @@ -163,14 +164,14 @@ func (whsvr *WebhookServer) getSidecarConfigurationRequested(ignoredList []strin glog.Infof("Pod %s/%s annotation %s is missing, skipping injection", metadata.Namespace, metadata.Name, requestAnnotationKey) return "", ErrMissingRequestAnnotation } - injectionKey := strings.ToLower(requestedInjection) - if !whsvr.Config.HasInjectionConfig(requestedInjection) { - glog.Errorf("Mutation policy for pod %s/%s: requested injection %s was not in configuration, skipping", metadata.Namespace, metadata.Name, requestedInjection) - return requestedInjection, ErrRequestedSidecarNotFound + ic, err := whsvr.Config.GetInjectionConfig(requestedInjection) + if err != nil { + glog.Errorf("Mutation policy for pod %s/%s: %v", metadata.Namespace, metadata.Name, err) + return "", ErrRequestedSidecarNotFound } - glog.Infof("Pod %s/%s annotation %s=%s requesting sidecar config %s", metadata.Namespace, metadata.Name, requestAnnotationKey, requestedInjection, injectionKey) - return injectionKey, nil + glog.Infof("Pod %s/%s annotation %s=%s requesting sidecar config %s", metadata.Namespace, metadata.Name, requestAnnotationKey, requestedInjection, ic.FullName()) + return ic.FullName(), nil } func setEnvironment(target []corev1.Container, addedEnv []corev1.EnvVar) (patch []patchOperation) { diff --git a/pkg/server/webhook_test.go b/pkg/server/webhook_test.go index 68ae260..c778e97 100644 --- a/pkg/server/webhook_test.go +++ b/pkg/server/webhook_test.go @@ -23,13 +23,16 @@ var ( // all these configs are deserialized into metav1.ObjectMeta structs obj1 = "test/fixtures/k8s/object1.yaml" - obj2 = "test/fixtures/k8s/object2.yaml" + obj2latest = "test/fixtures/k8s/object2latest.yaml" + obj2v = "test/fixtures/k8s/object2v.yaml" env1 = "test/fixtures/k8s/env1.yaml" obj3Missing = "test/fixtures/k8s/object3-missing.yaml" obj4 = "test/fixtures/k8s/object4.yaml" obj5 = "test/fixtures/k8s/object5.yaml" obj6 = "test/fixtures/k8s/object6.yaml" obj7 = "test/fixtures/k8s/object7.yaml" + obj7v2 = "test/fixtures/k8s/object7-v2.yaml" + obj7v3 = "test/fixtures/k8s/object7-badrequestformat.yaml" ignoredNamespace = "test/fixtures/k8s/ignored-namespace-pod.yaml" badSidecar = "test/fixtures/k8s/bad-sidecar.yaml" @@ -37,16 +40,19 @@ var ( // tests to check config loading of sidecars configTests = []expectedSidecarConfiguration{ - {configuration: obj1, expectedSidecar: "sidecar-test"}, - {configuration: obj2, expectedSidecar: "complex-sidecar"}, - {configuration: env1, expectedSidecar: "env1"}, + {configuration: obj1, expectedSidecar: "sidecar-test:latest"}, + {configuration: obj2latest, expectedSidecar: "", expectedError: ErrRequestedSidecarNotFound}, + {configuration: obj2v, expectedSidecar: "complex-sidecar:v420.69"}, + {configuration: env1, expectedSidecar: "env1:latest"}, {configuration: obj3Missing, expectedSidecar: "", expectedError: ErrMissingRequestAnnotation}, // this one is missing any annotations :) {configuration: obj4, expectedSidecar: "", expectedError: ErrSkipAlreadyInjected}, // this one is already injected, so it should not get injected again - {configuration: obj5, expectedSidecar: "volume-mounts"}, - {configuration: obj6, expectedSidecar: "host-aliases"}, - {configuration: obj7, expectedSidecar: "init-containers"}, + {configuration: obj5, expectedSidecar: "volume-mounts:latest"}, + {configuration: obj6, expectedSidecar: "host-aliases:latest"}, + {configuration: obj7, expectedSidecar: "init-containers:latest"}, + {configuration: obj7v2, expectedSidecar: "init-containers:v2"}, + {configuration: obj7v3, expectedSidecar: "", expectedError: ErrRequestedSidecarNotFound}, {configuration: ignoredNamespace, expectedSidecar: "", expectedError: ErrSkipIgnoredNamespace}, - {configuration: badSidecar, expectedSidecar: "this-doesnt-exist", expectedError: ErrRequestedSidecarNotFound}, + {configuration: badSidecar, expectedSidecar: "", expectedError: ErrRequestedSidecarNotFound}, } // tests to check the mutate() function for correct operation @@ -77,17 +83,14 @@ type mutationTest struct { func TestLoadConfig(t *testing.T) { c, err := config.LoadConfigDirectory(sidecars) if err != nil { - t.Error(err) - t.Fail() + t.Fatal(err) } c.AnnotationNamespace = "injector.unittest.com" if len(c.Injections) != expectedNumInjectionConfigs { - t.Errorf("expected %d injection configs to be loaded from %s, but got %d", expectedNumInjectionConfigs, sidecars, len(c.Injections)) - t.Fail() + t.Fatalf("expected %d injection configs to be loaded from %s, but got %d", expectedNumInjectionConfigs, sidecars, len(c.Injections)) } if c.AnnotationNamespace != "injector.unittest.com" { - t.Errorf("expected injector.unittest.com default AnnotationNamespace but got %s", c.AnnotationNamespace) - t.Fail() + t.Fatalf("expected injector.unittest.com default AnnotationNamespace but got %s", c.AnnotationNamespace) } s := &WebhookServer{ @@ -100,23 +103,19 @@ func TestLoadConfig(t *testing.T) { for _, test := range configTests { data, err := ioutil.ReadFile(test.configuration) if err != nil { - t.Errorf("unable to load object metadata yaml: %v", err) - t.Fail() + t.Fatalf("unable to load object metadata yaml: %v", err) } var obj *metav1.ObjectMeta if err := yaml.Unmarshal(data, &obj); err != nil { - t.Errorf("unable to unmarshal object metadata yaml: %v", err) - t.Fail() + t.Fatalf("unable to unmarshal object metadata yaml: %v", err) } key, err := s.getSidecarConfigurationRequested(testIgnoredNamespaces, obj) if err != test.expectedError { - t.Errorf("%s: error %v did not match %v", test.configuration, err, test.expectedError) - t.Fail() + t.Fatalf("%s: (expectedSidecar %s) error: %v did not match %v (k %v)", test.configuration, test.expectedSidecar, err, test.expectedError, key) } if key != test.expectedSidecar { - t.Errorf("%s: expected sidecar to be %v but was %v instead", test.configuration, test.expectedSidecar, key) - t.Fail() + t.Fatalf("%s: expected sidecar to be %v but was %v instead", test.configuration, test.expectedSidecar, key) } } @@ -145,12 +144,10 @@ func TestMutation(t *testing.T) { // load the AdmissionRequest object reqData, err := ioutil.ReadFile(reqFile) if err != nil { - t.Errorf("%s: unable to load AdmissionRequest object: %v", reqFile, err) - t.Fail() + t.Fatalf("%s: unable to load AdmissionRequest object: %v", reqFile, err) } if err := yaml.Unmarshal(reqData, &req); err != nil { - t.Errorf("%s: unable to unmarshal AdmissionRequest yaml: %v", reqFile, err) - t.Fail() + t.Fatalf("%s: unable to unmarshal AdmissionRequest yaml: %v", reqFile, err) } // stuff the request into mutate, and catch the response @@ -161,8 +158,7 @@ func TestMutation(t *testing.T) { res.Patch = nil // zero this field out if test.allowed != res.Allowed { - t.Errorf("expected AdmissionResponse.Allowed=%v differed from received AdmissionResponse.Allowed=%v", test.allowed, res.Allowed) - t.Fail() + t.Fatalf("expected AdmissionResponse.Allowed=%v differed from received AdmissionResponse.Allowed=%v", test.allowed, res.Allowed) } // diff the JSON patch object with expected JSON loaded from disk @@ -177,7 +173,7 @@ func TestMutation(t *testing.T) { } difference, diffString := jsondiff.Compare(expectedPatchData, resPatch, &jsondiffopts) if difference != jsondiff.FullMatch { - t.Errorf("received AdmissionResponse.patch field differed from expected with %s (%s) (actual on left, expected on right):\n%s", resPatchFile, difference.String(), diffString) + t.Fatalf("received AdmissionResponse.patch field differed from expected with %s (%s) (actual on left, expected on right):\n%s", resPatchFile, difference.String(), diffString) } } diff --git a/test/fixtures/k8s/configmap-complex-sidecar.yaml b/test/fixtures/k8s/configmap-complex-sidecar.yaml index 5b9f9d7..15b6f7e 100644 --- a/test/fixtures/k8s/configmap-complex-sidecar.yaml +++ b/test/fixtures/k8s/configmap-complex-sidecar.yaml @@ -6,7 +6,7 @@ metadata: namespace: default data: complex-sidecar: | - name: complex-sidecar + name: complex-sidecar:v420.69 volumes: - name: foo-config configMap: diff --git a/test/fixtures/k8s/object2.yaml b/test/fixtures/k8s/object2latest.yaml similarity index 100% rename from test/fixtures/k8s/object2.yaml rename to test/fixtures/k8s/object2latest.yaml diff --git a/test/fixtures/k8s/object2v.yaml b/test/fixtures/k8s/object2v.yaml new file mode 100644 index 0000000..1fa9383 --- /dev/null +++ b/test/fixtures/k8s/object2v.yaml @@ -0,0 +1,4 @@ +name: object2 +namespace: unittest +annotations: + "injector.unittest.com/request": "complex-sidecar:v420.69" diff --git a/test/fixtures/k8s/object7-badrequestformat.yaml b/test/fixtures/k8s/object7-badrequestformat.yaml new file mode 100644 index 0000000..97a9488 --- /dev/null +++ b/test/fixtures/k8s/object7-badrequestformat.yaml @@ -0,0 +1,4 @@ +name: object7v3 +namespace: unittest +annotations: + "injector.unittest.com/request": "init-containers:extra:data:v3" diff --git a/test/fixtures/k8s/object7-v2.yaml b/test/fixtures/k8s/object7-v2.yaml new file mode 100644 index 0000000..162c5f0 --- /dev/null +++ b/test/fixtures/k8s/object7-v2.yaml @@ -0,0 +1,4 @@ +name: object7v2 +namespace: unittest +annotations: + "injector.unittest.com/request": "init-containers:v2" diff --git a/test/fixtures/sidecars/bad/inheritance-escape.yaml b/test/fixtures/sidecars/bad/inheritance-escape.yaml new file mode 100644 index 0000000..9b95817 --- /dev/null +++ b/test/fixtures/sidecars/bad/inheritance-escape.yaml @@ -0,0 +1,4 @@ +name: inheritance-escape:v1 + +# try to escape +inherits: "../../etc/passwd" diff --git a/test/fixtures/sidecars/bad/inheritance-filenotfound.yaml b/test/fixtures/sidecars/bad/inheritance-filenotfound.yaml new file mode 100644 index 0000000..1adc833 --- /dev/null +++ b/test/fixtures/sidecars/bad/inheritance-filenotfound.yaml @@ -0,0 +1,4 @@ +name: inheritance-filenotfound + +inherits: "some-missing-file.yaml" + diff --git a/test/fixtures/sidecars/bad/init-containers-colons-v3.yaml b/test/fixtures/sidecars/bad/init-containers-colons-v3.yaml new file mode 100644 index 0000000..2ffbcb8 --- /dev/null +++ b/test/fixtures/sidecars/bad/init-containers-colons-v3.yaml @@ -0,0 +1,21 @@ +name: init-containers:extra:data:v3 +initContainers: + - name: init-container-1 + image: foo:bar1 + imagePullPolicy: Always + command: + - "bash" + - "-c" + - > + echo "sleep 20" && + sleep 20 +containers: +- name: sidecar-add-vm + image: nginx:1.12.2 + imagePullPolicy: IfNotPresent + ports: + - containerPort: 80 +- name: sidecar-existing-vm + image: foo:69 + ports: + - containerPort: 420 diff --git a/test/fixtures/sidecars/bad/missing-name.yaml b/test/fixtures/sidecars/bad/missing-name.yaml new file mode 100644 index 0000000..15be46f --- /dev/null +++ b/test/fixtures/sidecars/bad/missing-name.yaml @@ -0,0 +1,20 @@ +initContainers: + - name: init-container-1 + image: foo:bar1 + imagePullPolicy: Always + command: + - "bash" + - "-c" + - > + echo "sleep 20" && + sleep 20 +containers: +- name: sidecar-add-vm + image: nginx:1.12.2 + imagePullPolicy: IfNotPresent + ports: + - containerPort: 80 +- name: sidecar-existing-vm + image: foo:69 + ports: + - containerPort: 420 diff --git a/test/fixtures/sidecars/complex-sidecar.yaml b/test/fixtures/sidecars/complex-sidecar.yaml index 3e7814a..d7ff56b 100644 --- a/test/fixtures/sidecars/complex-sidecar.yaml +++ b/test/fixtures/sidecars/complex-sidecar.yaml @@ -1,5 +1,5 @@ --- -name: complex-sidecar +name: complex-sidecar:v420.69 volumes: - name: foo-config configMap: diff --git a/test/fixtures/sidecars/inheritance-1.yaml b/test/fixtures/sidecars/inheritance-1.yaml new file mode 100644 index 0000000..97beacf --- /dev/null +++ b/test/fixtures/sidecars/inheritance-1.yaml @@ -0,0 +1,42 @@ +name: inheritance-complex:v1 + +inherits: "complex-sidecar.yaml" + +volumes: + # existing volume + - name: foo-config + configMap: + name: foo-config + items: + - key: replace-me.yaml + path: replace-me.yaml + # new volume + - name: new-volume + configMap: + name: foo-config + items: + - key: replace-me.yaml + path: replace-me.yaml + +hostAliases: + - ip: 1.2.3.4 + hostnames: + - some.other-domain.com +env: + # existing env var, replace in place + - name: DATACENTER + value: new value, existing envvar + # brand new env var + - name: NEW_VARIABLE + value: test + +containers: + # an existing named image + - name: foo + image: nginx:69.420 + - name: a-new-image + image: some-value + +initContainers: + - name: a-new-image + image: some-value diff --git a/test/fixtures/sidecars/inheritance-deep-2.yaml b/test/fixtures/sidecars/inheritance-deep-2.yaml new file mode 100644 index 0000000..87f9640 --- /dev/null +++ b/test/fixtures/sidecars/inheritance-deep-2.yaml @@ -0,0 +1,45 @@ +name: inheritance-deep:v2 + +inherits: "inheritance-1.yaml" + +volumes: + # existing volume + - name: foo-config + configMap: + name: foo-config + items: + - key: replace-me.yaml + path: replace-me.yaml + # new volume + - name: new-volume-2 + configMap: + name: foo-config + items: + - key: replace-me.yaml + path: replace-me.yaml + +hostAliases: + - ip: 1.2.3.4 + hostnames: + - some.other-domain.com + - ip: 1.2.3.4 + hostnames: + - some.other-domain.com +env: + # existing env var, replace in place + - name: DATACENTER + value: from-inheritance-deep-2 + # brand new env var + - name: NEW_VAR_DEEP_V2 + value: from deep v2 + +containers: + # an existing named image + - name: foo + image: nginx:69.420 + - name: a-new-image-2 + image: some-value + +initContainers: + - name: a-new-image-2 + image: some-value diff --git a/test/fixtures/sidecars/init-containers-v2.yaml b/test/fixtures/sidecars/init-containers-v2.yaml new file mode 100644 index 0000000..5d3e6a7 --- /dev/null +++ b/test/fixtures/sidecars/init-containers-v2.yaml @@ -0,0 +1,21 @@ +name: init-containers:v2 +initContainers: + - name: init-container-1 + image: foo:bar1 + imagePullPolicy: Always + command: + - "bash" + - "-c" + - > + echo "sleep 20" && + sleep 20 +containers: +- name: sidecar-add-vm + image: nginx:1.12.2 + imagePullPolicy: IfNotPresent + ports: + - containerPort: 80 +- name: sidecar-existing-vm + image: foo:69 + ports: + - containerPort: 420