Skip to content

Commit

Permalink
Refactor handler webhook to be of type admission.Webhook
Browse files Browse the repository at this point in the history
- Replace pointer references with values refernces in methods used
  by connect-inject for Pods.
  • Loading branch information
Ashwin Venkatesh committed Mar 18, 2021
1 parent d88d866 commit 2d1c75b
Show file tree
Hide file tree
Showing 18 changed files with 615 additions and 1,031 deletions.
2 changes: 1 addition & 1 deletion connect-inject/consul_sidecar.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import (
corev1 "k8s.io/api/core/v1"
)

func (h *Handler) consulSidecar(pod *corev1.Pod) (corev1.Container, error) {
func (h *Handler) consulSidecar(pod corev1.Pod) (corev1.Container, error) {
command := []string{
"consul-k8s",
"consul-sidecar",
Expand Down
10 changes: 5 additions & 5 deletions connect-inject/consul_sidecar_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ func TestConsulSidecar_Default(t *testing.T) {
ImageConsulK8S: "hashicorp/consul-k8s:9.9.9",
ConsulSidecarResources: consulSidecarResources,
}
container, err := handler.consulSidecar(&corev1.Pod{
container, err := handler.consulSidecar(corev1.Pod{
Spec: corev1.PodSpec{
Containers: []corev1.Container{
{
Expand Down Expand Up @@ -84,7 +84,7 @@ func TestConsulSidecar_AuthMethod(t *testing.T) {
AuthMethod: authMethod,
ImageConsulK8S: "hashicorp/consul-k8s:9.9.9",
}
container, err := handler.consulSidecar(&corev1.Pod{
container, err := handler.consulSidecar(corev1.Pod{
Spec: corev1.PodSpec{
Containers: []corev1.Container{
{
Expand Down Expand Up @@ -113,7 +113,7 @@ func TestConsulSidecar_SyncPeriodAnnotation(t *testing.T) {
Log: hclog.Default().Named("handler"),
ImageConsulK8S: "hashicorp/consul-k8s:9.9.9",
}
container, err := handler.consulSidecar(&corev1.Pod{
container, err := handler.consulSidecar(corev1.Pod{
ObjectMeta: metav1.ObjectMeta{
Annotations: map[string]string{
"consul.hashicorp.com/connect-sync-period": "55s",
Expand Down Expand Up @@ -141,7 +141,7 @@ func TestConsulSidecar_TLS(t *testing.T) {
ConsulCACert: "consul-ca-cert",
ConsulSidecarResources: consulSidecarResources,
}
container, err := handler.consulSidecar(&corev1.Pod{
container, err := handler.consulSidecar(corev1.Pod{
Spec: corev1.PodSpec{
Containers: []corev1.Container{
{
Expand Down Expand Up @@ -194,7 +194,7 @@ func TestConsulSidecar_MetricsFlags(t *testing.T) {
DefaultEnableMetrics: true,
DefaultEnableMetricsMerging: true,
}
container, err := handler.consulSidecar(&corev1.Pod{
container, err := handler.consulSidecar(corev1.Pod{
ObjectMeta: metav1.ObjectMeta{
Annotations: map[string]string{
annotationMergedMetricsPort: "20100",
Expand Down
2 changes: 1 addition & 1 deletion connect-inject/container_env.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import (
corev1 "k8s.io/api/core/v1"
)

func (h *Handler) containerEnvVars(pod *corev1.Pod) []corev1.EnvVar {
func (h *Handler) containerEnvVars(pod corev1.Pod) []corev1.EnvVar {
raw, ok := pod.Annotations[annotationUpstreams]
if !ok || raw == "" {
return []corev1.EnvVar{}
Expand Down
2 changes: 1 addition & 1 deletion connect-inject/container_env_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ func TestContainerEnvVars(t *testing.T) {
require := require.New(t)

var h Handler
envVars := h.containerEnvVars(&corev1.Pod{
envVars := h.containerEnvVars(corev1.Pod{
ObjectMeta: metav1.ObjectMeta{
Annotations: map[string]string{
annotationService: "foo",
Expand Down
2 changes: 1 addition & 1 deletion connect-inject/container_init.go
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ func (h *Handler) containerInitCopyContainer() corev1.Container {

// containerInit returns the init container spec for registering the Consul
// service, setting up the Envoy bootstrap, etc.
func (h *Handler) containerInit(pod *corev1.Pod, k8sNamespace string) (corev1.Container, error) {
func (h *Handler) containerInit(pod corev1.Pod, k8sNamespace string) (corev1.Container, error) {
data := initContainerCommandData{
ServiceName: pod.Annotations[annotationService],
ProxyServiceName: fmt.Sprintf("%s-sidecar-proxy", pod.Annotations[annotationService]),
Expand Down
16 changes: 8 additions & 8 deletions connect-inject/container_init_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -671,7 +671,7 @@ services {
h := Handler{
ConsulClient: consulClient,
}
container, err := h.containerInit(tt.Pod(minimal()), k8sNamespace)
container, err := h.containerInit(*tt.Pod(minimal()), k8sNamespace)
require.NoError(err)
actual := strings.Join(container.Command, " ")
require.Contains(actual, tt.Cmd)
Expand Down Expand Up @@ -1100,7 +1100,7 @@ EOF
require.True(written)
h.ConsulClient = consulClient

container, err := h.containerInit(tt.Pod(minimal()), k8sNamespace)
container, err := h.containerInit(*tt.Pod(minimal()), k8sNamespace)
require.NoError(err)
actual := strings.Join(container.Command, " ")
require.Contains(actual, tt.Cmd)
Expand Down Expand Up @@ -1139,7 +1139,7 @@ func TestHandlerContainerInit_authMethod(t *testing.T) {
ServiceAccountName: "foo",
},
}
container, err := h.containerInit(pod, k8sNamespace)
container, err := h.containerInit(*pod, k8sNamespace)
require.NoError(err)
actual := strings.Join(container.Command, " ")
require.Contains(actual, `
Expand Down Expand Up @@ -1180,7 +1180,7 @@ func TestHandlerContainerInit_WithTLS(t *testing.T) {
},
},
}
container, err := h.containerInit(pod, k8sNamespace)
container, err := h.containerInit(*pod, k8sNamespace)
require.NoError(err)
actual := strings.Join(container.Command, " ")
require.Contains(actual, `
Expand Down Expand Up @@ -1224,7 +1224,7 @@ func TestHandlerContainerInit_Resources(t *testing.T) {
},
},
}
container, err := h.containerInit(pod, k8sNamespace)
container, err := h.containerInit(*pod, k8sNamespace)
require.NoError(err)
require.Equal(corev1.ResourceRequirements{
Limits: corev1.ResourceList{
Expand Down Expand Up @@ -1259,7 +1259,7 @@ func TestHandlerContainerInit_MismatchedServiceNameServiceAccountNameWithACLsEna
},
}

_, err := h.containerInit(pod, k8sNamespace)
_, err := h.containerInit(*pod, k8sNamespace)
require.EqualError(err, `serviceAccountName "notServiceName" does not match service name "foo"`)
}

Expand All @@ -1282,7 +1282,7 @@ func TestHandlerContainerInit_MismatchedServiceNameServiceAccountNameWithACLsDis
},
}

_, err := h.containerInit(pod, k8sNamespace)
_, err := h.containerInit(*pod, k8sNamespace)
require.NoError(err)
}

Expand Down Expand Up @@ -1407,7 +1407,7 @@ func TestHandlerContainerInit_MeshGatewayModeErrors(t *testing.T) {
},
},
}
_, err = h.containerInit(pod, k8sNamespace)
_, err = h.containerInit(*pod, k8sNamespace)
if c.ExpError == "" {
require.NoError(err)
} else {
Expand Down
6 changes: 3 additions & 3 deletions connect-inject/endpoints_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ func (r *EndpointsController) Reconcile(req ctrl.Request) (ctrl.Result, error) {
// and register that port for the host service.
var servicePort int
if raw, ok := pod.Annotations[annotationPort]; ok && raw != "" {
if port, _ := portValue(&pod, raw); port > 0 {
if port, _ := portValue(pod, raw); port > 0 {
servicePort = int(port)
}
}
Expand Down Expand Up @@ -184,7 +184,7 @@ func (r *EndpointsController) Reconcile(req ctrl.Request) (ctrl.Result, error) {
proxyConfig.LocalServicePort = servicePort
}

proxyConfig.Upstreams, err = r.processUpstreams(&pod)
proxyConfig.Upstreams, err = r.processUpstreams(pod)
if err != nil {
return ctrl.Result{}, err
}
Expand Down Expand Up @@ -328,7 +328,7 @@ func (r *EndpointsController) deregisterServiceOnAllAgents(k8sSvcName, k8sSvcNam

// processUpstreams reads the list of upstreams from the Pod annotation and converts them into a list of api.Upstream
// objects.
func (r *EndpointsController) processUpstreams(pod *corev1.Pod) ([]api.Upstream, error) {
func (r *EndpointsController) processUpstreams(pod corev1.Pod) ([]api.Upstream, error) {
var upstreams []api.Upstream
if raw, ok := pod.Annotations[annotationUpstreams]; ok && raw != "" {
for _, raw := range strings.Split(raw, ",") {
Expand Down
6 changes: 3 additions & 3 deletions connect-inject/envoy_sidecar.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ type sidecarContainerCommandData struct {
ConsulNamespace string
}

func (h *Handler) envoySidecar(pod *corev1.Pod, k8sNamespace string) (corev1.Container, error) {
func (h *Handler) envoySidecar(pod corev1.Pod, k8sNamespace string) (corev1.Container, error) {
templateData := sidecarContainerCommandData{
AuthMethod: h.AuthMethod,
ConsulNamespace: h.consulNamespace(k8sNamespace),
Expand Down Expand Up @@ -91,7 +91,7 @@ func (h *Handler) envoySidecar(pod *corev1.Pod, k8sNamespace string) (corev1.Con
}
return container, nil
}
func (h *Handler) getContainerSidecarCommand(pod *corev1.Pod) ([]string, error) {
func (h *Handler) getContainerSidecarCommand(pod corev1.Pod) ([]string, error) {
cmd := []string{
"envoy",
"--config-path", "/consul/connect-inject/envoy-bootstrap.yaml",
Expand Down Expand Up @@ -124,7 +124,7 @@ func (h *Handler) getContainerSidecarCommand(pod *corev1.Pod) ([]string, error)
return cmd, nil
}

func (h *Handler) envoySidecarResources(pod *corev1.Pod) (corev1.ResourceRequirements, error) {
func (h *Handler) envoySidecarResources(pod corev1.Pod) (corev1.ResourceRequirements, error) {
resources := corev1.ResourceRequirements{
Limits: corev1.ResourceList{},
Requests: corev1.ResourceList{},
Expand Down
14 changes: 7 additions & 7 deletions connect-inject/envoy_sidecar_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import (
func TestHandlerEnvoySidecar(t *testing.T) {
require := require.New(t)
h := Handler{}
pod := &corev1.Pod{
pod := corev1.Pod{
ObjectMeta: metav1.ObjectMeta{
Annotations: map[string]string{
annotationService: "foo",
Expand Down Expand Up @@ -144,7 +144,7 @@ func TestHandlerEnvoySidecar_EnvoyExtraArgs(t *testing.T) {
EnvoyExtraArgs: tc.envoyExtraArgs,
}

c, err := h.envoySidecar(tc.pod, k8sNamespace)
c, err := h.envoySidecar(*tc.pod, k8sNamespace)
require.NoError(t, err)
require.Equal(t, tc.expectedContainerCommand, c.Command)
})
Expand All @@ -158,7 +158,7 @@ func TestHandlerEnvoySidecar_AuthMethod(t *testing.T) {
h := Handler{
AuthMethod: "test-auth-method",
}
pod := &corev1.Pod{
pod := corev1.Pod{
ObjectMeta: metav1.ObjectMeta{
Annotations: map[string]string{
annotationService: "foo",
Expand Down Expand Up @@ -192,7 +192,7 @@ func TestHandlerEnvoySidecar_WithTLS(t *testing.T) {
h := Handler{
ConsulCACert: "consul-ca-cert",
}
pod := &corev1.Pod{
pod := corev1.Pod{
ObjectMeta: metav1.ObjectMeta{
Annotations: map[string]string{
annotationService: "foo",
Expand Down Expand Up @@ -237,7 +237,7 @@ func TestHandlerEnvoySidecar_Namespaces(t *testing.T) {
EnableNamespaces: true,
ConsulDestinationNamespace: k8sNamespace,
}
pod := &corev1.Pod{
pod := corev1.Pod{
ObjectMeta: metav1.ObjectMeta{
Annotations: map[string]string{
annotationService: "foo",
Expand Down Expand Up @@ -268,7 +268,7 @@ func TestHandlerEnvoySidecar_NamespacesAndAuthMethod(t *testing.T) {
ConsulDestinationNamespace: k8sNamespace,
AuthMethod: "test-auth-method",
}
pod := &corev1.Pod{
pod := corev1.Pod{
ObjectMeta: metav1.ObjectMeta{
Annotations: map[string]string{
annotationService: "foo",
Expand Down Expand Up @@ -449,7 +449,7 @@ func TestHandlerEnvoySidecar_Resources(t *testing.T) {
for name, c := range cases {
t.Run(name, func(tt *testing.T) {
require := require.New(tt)
pod := &corev1.Pod{
pod := corev1.Pod{
ObjectMeta: metav1.ObjectMeta{
Annotations: c.annotations,
},
Expand Down
Loading

0 comments on commit 2d1c75b

Please sign in to comment.