Namespace will now be extracted from request URL when not provided in…

… the body (#5064)

* Namespace will now be extracted from request URL when not provided in the body

Signed-off-by: Fabian von Feilitzsch <fabian@fabianism.us>

changelog/fragments/detect-request-ns.yaml

@@ -0,0 +1,8 @@
+entries:
+  - description: >
+      For Ansible-based operators, if a request is sent without a body in
+      the metadata it will now be extracted from the request URL and properly
+      set owner references/dependent watches.
+    kind: "bugfix"
+    breaking: false
+

internal/ansible/proxy/inject_owner.go

@@ -136,7 +136,7 @@ func (i *injectOwnerReferenceHandler) ServeHTTP(w http.ResponseWriter, req *http
 			ownerObject.SetGroupVersionKind(ownerGVK)
 			ownerObject.SetNamespace(owner.Namespace)
 			ownerObject.SetName(owner.Name)
-			addOwnerRef, err := k8sutil.SupportsOwnerReference(i.restMapper, ownerObject, data)
+			addOwnerRef, err := k8sutil.SupportsOwnerReference(i.restMapper, ownerObject, data, r.Namespace)
 			if err != nil {
 				m := "Could not determine if we should add owner ref"
 				log.Error(err, m)

internal/ansible/proxy/inject_owner_test.go

@@ -0,0 +1,125 @@
+// Copyright 2021 The Operator-SDK Authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package proxy
+
+import (
+	"bytes"
+	"context"
+	"encoding/json"
+	"fmt"
+	"io/ioutil"
+	"net/http"
+	"testing"
+
+	. "github.com/onsi/ginkgo"
+	. "github.com/onsi/gomega"
+
+	corev1 "k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+
+	"github.com/operator-framework/operator-sdk/internal/ansible/proxy/kubeconfig"
+)
+
+var _ = Describe("injectOwnerReferenceHandler", func() {
+
+	Describe("ServeHTTP", func() {
+		It("Should inject ownerReferences even when namespace is not explicitly set", func() {
+			if testing.Short() {
+				Skip("skipping ansible owner reference injection testing in short mode")
+			}
+			cm := corev1.ConfigMap{
+				TypeMeta: metav1.TypeMeta{
+					APIVersion: "v1",
+					Kind:       "ConfigMap",
+				},
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "test-owner-ref-injection",
+				},
+				Data: map[string]string{
+					"hello": "world",
+				},
+			}
+
+			body, err := json.Marshal(cm)
+			if err != nil {
+				Fail("Failed to marshal body")
+			}
+
+			po, err := createTestPod("test-injection", "default", testClient)
+			if err != nil {
+				Fail(fmt.Sprintf("Failed to create pod: %v", err))
+			}
+			defer func() {
+				if err := testClient.Delete(context.Background(), po); err != nil {
+					Fail(fmt.Sprintf("Failed to delete the pod: %v", err))
+				}
+			}()
+
+			req, err := http.NewRequest("POST", "http://localhost:8888/api/v1/namespaces/default/configmaps", bytes.NewReader(body))
+			if err != nil {
+				Fail(fmt.Sprintf("Failed to create http request: %v", err))
+			}
+
+			username, err := kubeconfig.EncodeOwnerRef(
+				metav1.OwnerReference{
+					APIVersion: "v1",
+					Kind:       "Pod",
+					Name:       po.GetName(),
+					UID:        po.GetUID(),
+				}, "default")
+			if err != nil {
+				Fail("Failed to encode owner reference")
+			}
+			req.SetBasicAuth(username, "unused")
+
+			httpClient := http.Client{}
+
+			defer func() {
+				cleanupReq, err := http.NewRequest("DELETE", "http://localhost:8888/api/v1/namespaces/default/configmaps/test-owner-ref-injection", bytes.NewReader([]byte{}))
+				if err != nil {
+					Fail(fmt.Sprintf("Failed to delete configmap: %v", err))
+				}
+				_, err = httpClient.Do(cleanupReq)
+				if err != nil {
+					Fail(fmt.Sprintf("Failed to delete configmap: %v", err))
+				}
+			}()
+
+			resp, err := httpClient.Do(req)
+			if err != nil {
+				Fail(fmt.Sprintf("Failed to create configmap: %v", err))
+			}
+			respBody, err := ioutil.ReadAll(resp.Body)
+			if err != nil {
+				Fail(fmt.Sprintf("Failed to read response body: %v", err))
+			}
+			var modifiedCM corev1.ConfigMap
+			err = json.Unmarshal(respBody, &modifiedCM)
+			if err != nil {
+				Fail(fmt.Sprintf("Failed to unmarshal configmap: %v", err))
+			}
+			ownerRefs := modifiedCM.ObjectMeta.OwnerReferences
+
+			Expect(len(ownerRefs)).To(Equal(1))
+
+			ownerRef := ownerRefs[0]
+
+			Expect(ownerRef.APIVersion).To(Equal("v1"))
+			Expect(ownerRef.Kind).To(Equal("Pod"))
+			Expect(ownerRef.Name).To(Equal(po.GetName()))
+			Expect(ownerRef.UID).To(Equal(po.GetUID()))
+		})
+	})
+})

internal/ansible/proxy/kubeconfig/kubeconfig.go

@@ -68,18 +68,28 @@ type NamespacedOwnerReference struct {
 	Namespace string
 }
 
+// EncodeOwnerRef takes an ownerReference and a namespace and returns a base64 encoded
+// string that can be used in the username field of a request to associate the
+// owner with the request being made.
+func EncodeOwnerRef(ownerRef metav1.OwnerReference, namespace string) (string, error) {
+	nsOwnerRef := NamespacedOwnerReference{OwnerReference: ownerRef, Namespace: namespace}
+	ownerRefJSON, err := json.Marshal(nsOwnerRef)
+	if err != nil {
+		return "", err
+	}
+	return base64.URLEncoding.EncodeToString(ownerRefJSON), nil
+}
+
 // Create renders a kubeconfig template and writes it to disk
 func Create(ownerRef metav1.OwnerReference, proxyURL string, namespace string) (*os.File, error) {
-	nsOwnerRef := NamespacedOwnerReference{OwnerReference: ownerRef, Namespace: namespace}
 	parsedURL, err := url.Parse(proxyURL)
 	if err != nil {
 		return nil, err
 	}
-	ownerRefJSON, err := json.Marshal(nsOwnerRef)
+	username, err := EncodeOwnerRef(ownerRef, namespace)
 	if err != nil {
 		return nil, err
 	}
-	username := base64.URLEncoding.EncodeToString(ownerRefJSON)
 	parsedURL.User = url.User(username)
 	v := values{
 		Username:  username,

internal/ansible/proxy/proxy_suite_test.go

@@ -0,0 +1,128 @@
+// Copyright 2021 The Operator-SDK Authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package proxy
+
+import (
+	"context"
+	"fmt"
+	"testing"
+
+	. "github.com/onsi/ginkgo"
+	. "github.com/onsi/gomega"
+	"github.com/operator-framework/operator-sdk/internal/ansible/proxy/controllermap"
+	kcorev1 "k8s.io/api/core/v1"
+	kmetav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"sigs.k8s.io/controller-runtime/pkg/client"
+	"sigs.k8s.io/controller-runtime/pkg/client/config"
+	"sigs.k8s.io/controller-runtime/pkg/manager"
+)
+
+var testMgr manager.Manager
+
+var testClient client.Client
+
+func TestProxy(t *testing.T) {
+	RegisterFailHandler(Fail)
+	RunSpecs(t, "Proxy Test Suite")
+}
+
+var _ = BeforeSuite(func() {
+	if testing.Short() {
+		return
+	}
+	var err error
+	testMgr, err = manager.New(config.GetConfigOrDie(), manager.Options{Namespace: "default"})
+	if err != nil {
+		Fail(fmt.Sprintf("Failed to instantiate manager: %v", err))
+	}
+	done := make(chan error)
+	cMap := controllermap.NewControllerMap()
+	err = Run(done, Options{
+		Address:           "localhost",
+		Port:              8888,
+		KubeConfig:        testMgr.GetConfig(),
+		Cache:             nil,
+		RESTMapper:        testMgr.GetRESTMapper(),
+		ControllerMap:     cMap,
+		WatchedNamespaces: []string{"test-watched-namespace"},
+		OwnerInjection:    true,
+	})
+	if err != nil {
+		Fail(fmt.Sprintf("Error starting proxy: %v", err))
+	}
+	testClient, err = client.New(testMgr.GetConfig(), client.Options{})
+	if err != nil {
+		Fail(fmt.Sprintf("Failed to create the client: %v", err))
+	}
+	_, err = createTestNamespace("test-watched-namespace", testClient)
+	if err != nil {
+		Fail(fmt.Sprintf("Failed to create watched namespace: %v", err))
+	}
+})
+
+var _ = AfterSuite(func() {
+	if testing.Short() {
+		return
+	}
+	err := testClient.Delete(context.Background(), &kcorev1.Namespace{
+		ObjectMeta: kmetav1.ObjectMeta{
+			Name: "test-watched-namespace",
+			Labels: map[string]string{
+				"test-label": "test-watched-namespace",
+			},
+		},
+	})
+
+	if err != nil {
+		Fail(fmt.Sprintf("Failed to clean up namespace: %v:", err))
+	}
+})
+
+func createTestNamespace(name string, cl client.Client) (client.Object, error) {
+	ns := &kcorev1.Namespace{
+		ObjectMeta: kmetav1.ObjectMeta{
+			Name: name,
+			Labels: map[string]string{
+				"test-label": name,
+			},
+		},
+	}
+	if err := cl.Create(context.Background(), ns); err != nil {
+		return nil, err
+	}
+	return ns, nil
+}
+
+func createTestPod(name, namespace string, cl client.Client) (client.Object, error) {
+	three := int64(3)
+	pod := &kcorev1.Pod{
+		ObjectMeta: kmetav1.ObjectMeta{
+			Name:      name,
+			Namespace: namespace,
+			Labels: map[string]string{
+				"test-label": name,
+			},
+		},
+		Spec: kcorev1.PodSpec{
+			Containers:            []kcorev1.Container{{Name: "nginx", Image: "nginx"}},
+			RestartPolicy:         "Always",
+			ActiveDeadlineSeconds: &three,
+		},
+	}
+	if err := cl.Create(context.Background(), pod); err != nil {
+		return nil, err
+	}
+	return pod, nil
+}