Skip to content

Commit

Permalink
added an error check to the containerEnvVars(pod) call
Browse files Browse the repository at this point in the history 
- added test cases
  • Loading branch information
@wilkermichael
wilkermichael committed Sep 22, 2023
1 parent 0e2cce7 commit ff506eb
Show file tree
Hide file tree
Showing 3 changed files with 91 additions and 64 deletions.
6 changes: 3 additions & 3 deletions control-plane/connect-inject/constants/constants.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ const (
)

// GetDefaultConsulNamespace returns the default namespace if the passed namespace
// is empty, otherwise returns back the passed in namespace
// is empty, otherwise returns back the passed in namespace.
func GetDefaultConsulNamespace(ns string) string {
if ns == "" {
ns = DefaultConsulNS
Expand All @@ -66,7 +66,7 @@ func GetDefaultConsulNamespace(ns string) string {
}

// GetDefaultConsulPartition returns the default partition if the passed partition
// is empty, otherwise returns back the passed in partition
// is empty, otherwise returns back the passed in partition.
func GetDefaultConsulPartition(ap string) string {
if ap == "" {
ap = DefaultConsulPartition
Expand All @@ -76,7 +76,7 @@ func GetDefaultConsulPartition(ap string) string {
}

// GetDefaultConsulPeer returns the default peer if the passed peer
// is empty, otherwise returns back the passed in peer
// is empty, otherwise returns back the passed in peer.
func GetDefaultConsulPeer(peer string) string {
if peer == "" {
peer = DefaultConsulPeer
Expand Down
6 changes: 5 additions & 1 deletion control-plane/connect-inject/webhook_v2/mesh_webhook.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -270,7 +270,11 @@ func (w *MeshWebhook) Handle(ctx context.Context, req admission.Request) admissi

// Add the upstream services as environment variables for easy
// service discovery.
containerEnvVars := w.containerEnvVars(pod)
containerEnvVars, err := w.containerEnvVars(pod)
if err != nil {
w.Log.Error(err, "error creating the port environment variables based on pod annotations", "request name", req.Name)
return admission.Errored(http.StatusInternalServerError, fmt.Errorf("error creating the port environment variables based on pod annotations: %s", err))
}
for i := range pod.Spec.InitContainers {
pod.Spec.InitContainers[i].Env = append(pod.Spec.InitContainers[i].Env, containerEnvVars...)
}
Expand Down
143 changes: 83 additions & 60 deletions control-plane/connect-inject/webhook_v2/mesh_webhook_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -209,66 +209,89 @@ func TestHandlerHandle(t *testing.T) {
},
},
},
// (TODO: ashwin) fix this test once upstreams get correctly processed
//{
// "pod with upstreams specified",
// MeshWebhook{
// Log: logrtest.New(t),
// AllowK8sNamespacesSet: mapset.NewSetWith("*"),
// DenyK8sNamespacesSet: mapset.NewSet(),
// decoder: decoder,
// Clientset: defaultTestClientWithNamespace(),
// },
// admission.Request{
// AdmissionRequest: admissionv1.AdmissionRequest{
// Namespace: namespaces.DefaultNamespace,
// Object: encodeRaw(t, &corev1.Pod{
// ObjectMeta: metav1.ObjectMeta{
// Annotations: map[string]string{
// constants.AnnotationMeshDestinations: "echo:1234,db:1234",
// },
// },
// Spec: basicSpec,
// }),
// },
// },
// "",
// []jsonpatch.Operation{
// {
// Operation: "add",
// Path: "/metadata/labels",
// },
// {
// Operation: "add",
// Path: "/metadata/annotations/" + escapeJSONPointer(constants.KeyMeshInjectStatus),
// },
// {
// Operation: "add",
// Path: "/metadata/annotations/" + escapeJSONPointer(constants.AnnotationOriginalPod),
// },
// {
// Operation: "add",
// Path: "/metadata/annotations/" + escapeJSONPointer(constants.AnnotationConsulK8sVersion),
// },
// {
// Operation: "add",
// Path: "/spec/volumes",
// },
// {
// Operation: "add",
// Path: "/spec/initContainers",
// },
// {
// Operation: "add",
// Path: "/spec/containers/1",
// },
// {
// Operation: "add",
// Path: "/spec/containers/0/env",
// },
// },
//},

{
"pod with upstreams specified",
MeshWebhook{
Log: logrtest.New(t),
AllowK8sNamespacesSet: mapset.NewSetWith("*"),
DenyK8sNamespacesSet: mapset.NewSet(),
decoder: decoder,
Clientset: defaultTestClientWithNamespace(),
},
admission.Request{
AdmissionRequest: admissionv1.AdmissionRequest{
Namespace: namespaces.DefaultNamespace,
Object: encodeRaw(t, &corev1.Pod{
ObjectMeta: metav1.ObjectMeta{
Annotations: map[string]string{
constants.AnnotationMeshDestinations: "myPort1.echo:1234,myPort2.db:1234",
},
},
Spec: basicSpec,
}),
},
},
"",
[]jsonpatch.Operation{
{
Operation: "add",
Path: "/metadata/labels",
},
{
Operation: "add",
Path: "/metadata/annotations/" + escapeJSONPointer(constants.KeyMeshInjectStatus),
},
{
Operation: "add",
Path: "/metadata/annotations/" + escapeJSONPointer(constants.AnnotationOriginalPod),
},
{
Operation: "add",
Path: "/metadata/annotations/" + escapeJSONPointer(constants.AnnotationConsulK8sVersion),
},
{
Operation: "add",
Path: "/spec/volumes",
},
{
Operation: "add",
Path: "/spec/initContainers",
},
{
Operation: "add",
Path: "/spec/containers/1",
},
{
Operation: "add",
Path: "/spec/containers/0/env",
},
},
},
{
"error pod with incorrect upstreams specified",
MeshWebhook{
Log: logrtest.New(t),
AllowK8sNamespacesSet: mapset.NewSetWith("*"),
DenyK8sNamespacesSet: mapset.NewSet(),
decoder: decoder,
Clientset: defaultTestClientWithNamespace(),
},
admission.Request{
AdmissionRequest: admissionv1.AdmissionRequest{
Namespace: namespaces.DefaultNamespace,
Object: encodeRaw(t, &corev1.Pod{
ObjectMeta: metav1.ObjectMeta{
Annotations: map[string]string{
constants.AnnotationMeshDestinations: "db:1234",
},
},
Spec: basicSpec,
}),
},
},
"error creating the port environment variables based on pod annotations",
[]jsonpatch.Operation{},
},
{
"empty pod with injection disabled",
MeshWebhook{
Expand Down

0 comments on commit ff506eb

Please sign in to comment.