address comments

cmd/main.go

@@ -129,7 +129,7 @@ func main() {
 				}
 				glog.V(1).Infof("got %d updated InjectionConfigs from reconciliation", len(updatedInjectionConfigs))
 
-				newInjectionConfigs := make([]config.InjectionConfig, len(updatedInjectionConfigs)+len(cfg.Injections))
+				newInjectionConfigs := make([]*config.InjectionConfig, len(updatedInjectionConfigs)+len(cfg.Injections))
 				{
 					i := 0
 					for k := range cfg.Injections {

internal/pkg/config/config.go

@@ -40,8 +40,8 @@ type InjectionConfig struct {
 // Config is a struct indicating how a given injection should be configured
 type Config struct {
 	sync.RWMutex
-	AnnotationNamespace string                     `yaml:"annotationnamespace"`
-	Injections          map[string]InjectionConfig `yaml:"injections"`
+	AnnotationNamespace string                      `yaml:"annotationnamespace"`
+	Injections          map[string]*InjectionConfig `yaml:"injections"`
 }
 
 // String returns a string representation of the config
@@ -51,10 +51,10 @@ func (c *InjectionConfig) String() string {
 
 // ReplaceInjectionConfigs will take a list of new InjectionConfigs, and replace the current configuration with them.
 // this blocks waiting on being able to update the configs in place.
-func (c *Config) ReplaceInjectionConfigs(replacementConfigs []InjectionConfig) {
+func (c *Config) ReplaceInjectionConfigs(replacementConfigs []*InjectionConfig) {
 	c.Lock()
 	defer c.Unlock()
-	c.Injections = map[string]InjectionConfig{}
+	c.Injections = map[string]*InjectionConfig{}
 	for _, r := range replacementConfigs {
 		c.Injections[r.Name] = r
 	}
@@ -78,13 +78,13 @@ func (c *Config) GetInjectionConfig(key string) (*InjectionConfig, error) {
 	if !ok {
 		return nil, fmt.Errorf("no injection config found for annotation %s", key)
 	}
-	return &i, nil
+	return i, nil
 }
 
 // LoadConfigDirectory loads all configs in a directory and returns the Config
 func LoadConfigDirectory(path string) (*Config, error) {
 	cfg := Config{
-		Injections: map[string]InjectionConfig{},
+		Injections: map[string]*InjectionConfig{},
 	}
 	glob := filepath.Join(path, "*.yaml")
 	matches, err := filepath.Glob(glob)
@@ -97,7 +97,7 @@ func LoadConfigDirectory(path string) (*Config, error) {
 			glog.Errorf("Error reading injection config from %s: %v", p, err)
 			return nil, err
 		}
-		cfg.Injections[c.Name] = *c
+		cfg.Injections[c.Name] = c
 	}
 
 	if len(cfg.Injections) == 0 {

internal/pkg/config/watcher/watcher.go

@@ -135,7 +135,7 @@ func mapStringStringToLabelSelector(m map[string]string) string {
 }
 
 // Get fetches all matching ConfigMaps
-func (c *K8sConfigMapWatcher) Get() (cfgs []config.InjectionConfig, err error) {
+func (c *K8sConfigMapWatcher) Get() (cfgs []*config.InjectionConfig, err error) {
 	glog.V(1).Infof("Fetching ConfigMaps...")
 	clist, err := c.client.ConfigMaps(c.Namespace).List(metav1.ListOptions{
 		LabelSelector: mapStringStringToLabelSelector(c.ConfigMapLabels),
@@ -156,16 +156,16 @@ func (c *K8sConfigMapWatcher) Get() (cfgs []config.InjectionConfig, err error) {
 }
 
 // InjectionConfigsFromConfigMap parse items in a configmap into a list of InjectionConfigs
-func InjectionConfigsFromConfigMap(cm v1.ConfigMap) ([]config.InjectionConfig, error) {
-	ics := []config.InjectionConfig{}
+func InjectionConfigsFromConfigMap(cm v1.ConfigMap) ([]*config.InjectionConfig, error) {
+	ics := []*config.InjectionConfig{}
 	for name, payload := range cm.Data {
 		glog.V(3).Infof("Parsing %s/%s:%s into InjectionConfig", cm.ObjectMeta.Namespace, cm.ObjectMeta.Name, name)
 		ic, err := config.LoadInjectionConfig(strings.NewReader(payload))
 		if err != nil {
 			return nil, fmt.Errorf("error parsing ConfigMap %s item %s into injection config: %s", cm.ObjectMeta.Name, name, err.Error())
 		}
 		glog.V(2).Infof("Loaded InjectionConfig %s from ConfigMap %s:%s", ic.Name, cm.ObjectMeta.Name, name)
-		ics = append(ics, *ic)
+		ics = append(ics, ic)
 	}
 	return ics, nil
 }

pkg/server/errors.go

@@ -14,3 +14,24 @@ var (
 	// ErrRequestedSidecarNotFound ...
 	ErrRequestedSidecarNotFound = fmt.Errorf("Requested sidecar not found in configuration")
 )
+
+// GetErrorReason returns a string description for a given error, for use
+// when reporting "reason" in metrics
+func GetErrorReason(err error) string {
+	var reason string
+	switch err {
+	case ErrSkipIgnoredNamespace:
+		reason = "ignored_namespace"
+	case ErrSkipAlreadyInjected:
+		reason = "already_injected"
+	case ErrMissingRequestAnnotation:
+		reason = "no_annotation"
+	case ErrRequestedSidecarNotFound:
+		reason = "missing_config"
+	case nil:
+		reason = ""
+	default:
+		reason = "unknown_error"
+	}
+	return reason
+}

pkg/server/webhook.go

@@ -19,14 +19,16 @@ import (
 	"k8s.io/apimachinery/pkg/runtime/serializer"
 )
 
+const (
+	// StatusInjected is the annotation value for /status that indicates an injection was already performed on this pod
+	StatusInjected = "injected"
+)
+
 var (
 	runtimeScheme = runtime.NewScheme()
 	codecs        = serializer.NewCodecFactory(runtimeScheme)
 	deserializer  = codecs.UniversalDeserializer()
 
-	// StatusInjected is the annotation value for /status that indicates an injection was already performed on this pod
-	StatusInjected = "injected"
-
 	// (https://github.com/kubernetes/kubernetes/issues/57982)
 	defaulter = runtime.ObjectDefaulter(runtimeScheme)
 
@@ -383,19 +385,7 @@ func (whsvr *WebhookServer) mutate(ar *v1beta1.AdmissionReview) *v1beta1.Admissi
 	injectionKey, err := whsvr.getSidecarConfigurationRequested(ignoredNamespaces, &pod.ObjectMeta)
 	if err != nil {
 		glog.Infof("Skipping mutation of %s/%s: %v", pod.Namespace, pod.Name, err)
-		var reason string
-		switch err {
-		case ErrSkipIgnoredNamespace:
-			reason = "ignored_namespace"
-		case ErrSkipAlreadyInjected:
-			reason = "already_injected"
-		case ErrMissingRequestAnnotation:
-			reason = "no_annotation"
-		case ErrRequestedSidecarNotFound:
-			reason = "missing_config"
-		default:
-			reason = "unknown_error"
-		}
+		reason := GetErrorReason(err)
 		injectionCounter.With(prometheus.Labels{"status": "skipped", "reason": reason, "requested": injectionKey}).Inc()
 		return &v1beta1.AdmissionResponse{
 			Allowed: true,