From a2ea804d2c7b9a70c8867a12a1834f488d490165 Mon Sep 17 00:00:00 2001 From: Nitishkumar Singh Date: Sun, 7 Nov 2021 10:01:19 +0530 Subject: [PATCH] Handled list secrets for no secret in namespaces Signed-off-by: Nitishkumar Singh Test case included for default and non-default Signed-off-by: Nitishkumar Singh Changed Fake Labeller Implementation Signed-off-by: Nitishkumar Singh --- cmd/provider.go | 2 +- pkg/provider/handlers/delete.go | 2 +- pkg/provider/handlers/deploy.go | 2 +- pkg/provider/handlers/functions.go | 2 +- pkg/provider/handlers/read.go | 5 +- pkg/provider/handlers/replicas.go | 2 +- pkg/provider/handlers/scale.go | 2 +- pkg/provider/handlers/secret.go | 29 +++++---- pkg/provider/handlers/secret_test.go | 92 ++++++++++++++++++++++++++++ pkg/provider/handlers/update.go | 2 +- pkg/provider/handlers/utils.go | 6 +- pkg/provider/labeller.go | 18 ++++++ 12 files changed, 140 insertions(+), 24 deletions(-) create mode 100644 pkg/provider/labeller.go diff --git a/cmd/provider.go b/cmd/provider.go index 00051c8..4759456 100644 --- a/cmd/provider.go +++ b/cmd/provider.go @@ -103,7 +103,7 @@ func makeProviderCmd() *cobra.Command { HealthHandler: func(w http.ResponseWriter, r *http.Request) {}, InfoHandler: handlers.MakeInfoHandler(Version, GitCommit), ListNamespaceHandler: handlers.MakeNamespacesLister(client), - SecretHandler: handlers.MakeSecretHandler(client, baseUserSecretsPath), + SecretHandler: handlers.MakeSecretHandler(client.NamespaceService(), baseUserSecretsPath), LogHandler: logs.NewLogHandlerFunc(faasdlogs.New(), config.ReadTimeout), } diff --git a/pkg/provider/handlers/delete.go b/pkg/provider/handlers/delete.go index 2fd8837..d45dfba 100644 --- a/pkg/provider/handlers/delete.go +++ b/pkg/provider/handlers/delete.go @@ -43,7 +43,7 @@ func MakeDeleteHandler(client *containerd.Client, cni gocni.CNI) func(w http.Res lookupNamespace := getRequestNamespace(readNamespaceFromQuery(r)) // Check if namespace exists, and it has the openfaas label - valid, err := validNamespace(client, lookupNamespace) + valid, err := validNamespace(client.NamespaceService(), lookupNamespace) if err != nil { http.Error(w, err.Error(), http.StatusBadRequest) return diff --git a/pkg/provider/handlers/deploy.go b/pkg/provider/handlers/deploy.go index 9b3eeb1..a0c174c 100644 --- a/pkg/provider/handlers/deploy.go +++ b/pkg/provider/handlers/deploy.go @@ -54,7 +54,7 @@ func MakeDeployHandler(client *containerd.Client, cni gocni.CNI, secretMountPath namespace := getRequestNamespace(req.Namespace) // Check if namespace exists, and it has the openfaas label - valid, err := validNamespace(client, namespace) + valid, err := validNamespace(client.NamespaceService(), namespace) if err != nil { http.Error(w, err.Error(), http.StatusBadRequest) diff --git a/pkg/provider/handlers/functions.go b/pkg/provider/handlers/functions.go index f76cbee..18c12c0 100644 --- a/pkg/provider/handlers/functions.go +++ b/pkg/provider/handlers/functions.go @@ -37,7 +37,7 @@ type Function struct { func ListFunctions(client *containerd.Client, namespace string) (map[string]*Function, error) { // Check if namespace exists, and it has the openfaas label - valid, err := validNamespace(client, namespace) + valid, err := validNamespace(client.NamespaceService(), namespace) if err != nil { return nil, err } diff --git a/pkg/provider/handlers/read.go b/pkg/provider/handlers/read.go index f487866..0421b5c 100644 --- a/pkg/provider/handlers/read.go +++ b/pkg/provider/handlers/read.go @@ -2,10 +2,11 @@ package handlers import ( "encoding/json" - "k8s.io/apimachinery/pkg/api/resource" "log" "net/http" + "k8s.io/apimachinery/pkg/api/resource" + "github.com/containerd/containerd" "github.com/openfaas/faas-provider/types" ) @@ -16,7 +17,7 @@ func MakeReadHandler(client *containerd.Client) func(w http.ResponseWriter, r *h lookupNamespace := getRequestNamespace(readNamespaceFromQuery(r)) // Check if namespace exists, and it has the openfaas label - valid, err := validNamespace(client, lookupNamespace) + valid, err := validNamespace(client.NamespaceService(), lookupNamespace) if err != nil { http.Error(w, err.Error(), http.StatusBadRequest) return diff --git a/pkg/provider/handlers/replicas.go b/pkg/provider/handlers/replicas.go index 81503ea..cfbcd0b 100644 --- a/pkg/provider/handlers/replicas.go +++ b/pkg/provider/handlers/replicas.go @@ -17,7 +17,7 @@ func MakeReplicaReaderHandler(client *containerd.Client) func(w http.ResponseWri lookupNamespace := getRequestNamespace(readNamespaceFromQuery(r)) // Check if namespace exists, and it has the openfaas label - valid, err := validNamespace(client, lookupNamespace) + valid, err := validNamespace(client.NamespaceService(), lookupNamespace) if err != nil { http.Error(w, err.Error(), http.StatusBadRequest) return diff --git a/pkg/provider/handlers/scale.go b/pkg/provider/handlers/scale.go index f705dc2..725341c 100644 --- a/pkg/provider/handlers/scale.go +++ b/pkg/provider/handlers/scale.go @@ -42,7 +42,7 @@ func MakeReplicaUpdateHandler(client *containerd.Client, cni gocni.CNI) func(w h namespace := getRequestNamespace(readNamespaceFromQuery(r)) // Check if namespace exists, and it has the openfaas label - valid, err := validNamespace(client, namespace) + valid, err := validNamespace(client.NamespaceService(), namespace) if err != nil { http.Error(w, err.Error(), http.StatusBadRequest) return diff --git a/pkg/provider/handlers/secret.go b/pkg/provider/handlers/secret.go index 009f22d..956668f 100644 --- a/pkg/provider/handlers/secret.go +++ b/pkg/provider/handlers/secret.go @@ -10,14 +10,14 @@ import ( "path" "strings" - "github.com/containerd/containerd" "github.com/openfaas/faas-provider/types" + provider "github.com/openfaas/faasd/pkg/provider" ) const secretFilePermission = 0644 const secretDirPermission = 0755 -func MakeSecretHandler(c *containerd.Client, mountPath string) func(w http.ResponseWriter, r *http.Request) { +func MakeSecretHandler(store provider.Labeller, mountPath string) func(w http.ResponseWriter, r *http.Request) { err := os.MkdirAll(mountPath, secretFilePermission) if err != nil { @@ -31,13 +31,13 @@ func MakeSecretHandler(c *containerd.Client, mountPath string) func(w http.Respo switch r.Method { case http.MethodGet: - listSecrets(c, w, r, mountPath) + listSecrets(store, w, r, mountPath) case http.MethodPost: - createSecret(c, w, r, mountPath) + createSecret(w, r, mountPath) case http.MethodPut: - createSecret(c, w, r, mountPath) + createSecret(w, r, mountPath) case http.MethodDelete: - deleteSecret(c, w, r, mountPath) + deleteSecret(w, r, mountPath) default: w.WriteHeader(http.StatusBadRequest) return @@ -46,11 +46,11 @@ func MakeSecretHandler(c *containerd.Client, mountPath string) func(w http.Respo } } -func listSecrets(c *containerd.Client, w http.ResponseWriter, r *http.Request, mountPath string) { +func listSecrets(store provider.Labeller, w http.ResponseWriter, r *http.Request, mountPath string) { lookupNamespace := getRequestNamespace(readNamespaceFromQuery(r)) // Check if namespace exists, and it has the openfaas label - valid, err := validNamespace(c, lookupNamespace) + valid, err := validNamespace(store, lookupNamespace) if err != nil { http.Error(w, err.Error(), http.StatusBadRequest) return @@ -63,8 +63,15 @@ func listSecrets(c *containerd.Client, w http.ResponseWriter, r *http.Request, m mountPath = getNamespaceSecretMountPath(mountPath, lookupNamespace) - files, err := ioutil.ReadDir(mountPath) + files, err := os.ReadDir(mountPath) + if os.IsNotExist(err) { + bytesOut, _ := json.Marshal([]types.Secret{}) + w.Write(bytesOut) + return + } + if err != nil { + fmt.Printf("Error Occured: %s \n", err) http.Error(w, err.Error(), http.StatusInternalServerError) return } @@ -78,7 +85,7 @@ func listSecrets(c *containerd.Client, w http.ResponseWriter, r *http.Request, m w.Write(bytesOut) } -func createSecret(c *containerd.Client, w http.ResponseWriter, r *http.Request, mountPath string) { +func createSecret(w http.ResponseWriter, r *http.Request, mountPath string) { secret, err := parseSecret(r) if err != nil { log.Printf("[secret] error %s", err.Error()) @@ -118,7 +125,7 @@ func createSecret(c *containerd.Client, w http.ResponseWriter, r *http.Request, } } -func deleteSecret(c *containerd.Client, w http.ResponseWriter, r *http.Request, mountPath string) { +func deleteSecret(w http.ResponseWriter, r *http.Request, mountPath string) { secret, err := parseSecret(r) if err != nil { log.Printf("[secret] error %s", err.Error()) diff --git a/pkg/provider/handlers/secret_test.go b/pkg/provider/handlers/secret_test.go index 2b3d8d8..6c73951 100644 --- a/pkg/provider/handlers/secret_test.go +++ b/pkg/provider/handlers/secret_test.go @@ -1,6 +1,9 @@ package handlers import ( + "encoding/json" + "fmt" + "io/ioutil" "net/http" "net/http/httptest" "os" @@ -10,6 +13,8 @@ import ( "testing" "github.com/openfaas/faas-provider/types" + "github.com/openfaas/faasd/pkg" + mock "github.com/openfaas/faasd/pkg/provider" ) func Test_parseSecret(t *testing.T) { @@ -161,3 +166,90 @@ func TestSecretCreation(t *testing.T) { }) } } + +func TestListSecrets(t *testing.T) { + mountPath, err := os.MkdirTemp("", "test_secret_creation") + if err != nil { + t.Fatalf("unexpected error while creating temp directory: %s", err) + } + + defer os.RemoveAll(mountPath) + + cases := []struct { + name string + verb string + namespace string + labels map[string]string + status int + secretPath string + secret string + err string + expected []types.Secret + }{ + { + name: "Get empty secret list for default namespace having no secret", + verb: http.MethodGet, + status: http.StatusOK, + secretPath: "/test-fn/foo", + secret: "bar", + expected: make([]types.Secret, 0), + }, + { + name: "Get empty secret list for non-default namespace having no secret", + verb: http.MethodGet, + status: http.StatusOK, + secretPath: "/test-fn/foo", + secret: "bar", + expected: make([]types.Secret, 0), + namespace: "other-ns", + labels: map[string]string{ + pkg.NamespaceLabel: "true", + }, + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + fakeStore := &mock.FakeLabeller{} + fakeStore.FakeLabels = tc.labels + + handler := MakeSecretHandler(fakeStore, mountPath) + + path := "http://example.com/foo" + if len(tc.namespace) > 0 { + path = path + fmt.Sprintf("?namespace=%s", tc.namespace) + } + req := httptest.NewRequest(tc.verb, path, nil) + w := httptest.NewRecorder() + + handler(w, req) + + resp := w.Result() + if resp.StatusCode != tc.status { + t.Logf("response body: %s", w.Body.String()) + t.Fatalf("expected status: %d, got: %d", tc.status, resp.StatusCode) + } + + if resp.StatusCode != http.StatusOK && w.Body.String() != tc.err { + t.Fatalf("expected error message: %q, got %q", tc.err, w.Body.String()) + + } + + body, err := ioutil.ReadAll(resp.Body) + if err != nil { + t.Fatalf("can not read response of list %v", err) + } + + var res []types.Secret + err = json.Unmarshal(body, &res) + + if err != nil { + t.Fatalf("can not read response of list %v", err) + } + + if !reflect.DeepEqual(res, tc.expected) { + t.Fatalf("expected response: %v, got: %v", tc.expected, res) + } + }) + } +} diff --git a/pkg/provider/handlers/update.go b/pkg/provider/handlers/update.go index da0695f..3461507 100644 --- a/pkg/provider/handlers/update.go +++ b/pkg/provider/handlers/update.go @@ -43,7 +43,7 @@ func MakeUpdateHandler(client *containerd.Client, cni gocni.CNI, secretMountPath namespace := getRequestNamespace(req.Namespace) // Check if namespace exists, and it has the openfaas label - valid, err := validNamespace(client, namespace) + valid, err := validNamespace(client.NamespaceService(), namespace) if err != nil { http.Error(w, err.Error(), http.StatusBadRequest) return diff --git a/pkg/provider/handlers/utils.go b/pkg/provider/handlers/utils.go index 5924be9..b95dc93 100644 --- a/pkg/provider/handlers/utils.go +++ b/pkg/provider/handlers/utils.go @@ -5,10 +5,9 @@ import ( "net/http" "path" - "github.com/containerd/containerd" - "github.com/openfaas/faasd/pkg" faasd "github.com/openfaas/faasd/pkg" + provider "github.com/openfaas/faasd/pkg/provider" ) func getRequestNamespace(namespace string) string { @@ -30,12 +29,11 @@ func getNamespaceSecretMountPath(userSecretPath string, namespace string) string // validNamespace indicates whether the namespace is eligable to be // used for OpenFaaS functions. -func validNamespace(client *containerd.Client, namespace string) (bool, error) { +func validNamespace(store provider.Labeller, namespace string) (bool, error) { if namespace == faasd.DefaultFunctionNamespace { return true, nil } - store := client.NamespaceService() labels, err := store.Labels(context.Background(), namespace) if err != nil { return false, err diff --git a/pkg/provider/labeller.go b/pkg/provider/labeller.go new file mode 100644 index 0000000..7b91383 --- /dev/null +++ b/pkg/provider/labeller.go @@ -0,0 +1,18 @@ +package provider + +import "context" + +type Labeller interface { + Labels(ctx context.Context, namespace string) (map[string]string, error) +} + +/* + * FakeLabeller can be used to fake labels applied on namespace to mark them valid/invalid for openfaas functions + */ +type FakeLabeller struct { + FakeLabels map[string]string +} + +func (s *FakeLabeller) Labels(ctx context.Context, namespace string) (map[string]string, error) { + return s.FakeLabels, nil +}