Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add multiple namespaces in the operator #673

Merged
merged 1 commit into from
Aug 21, 2020
Merged

Add multiple namespaces in the operator #673

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions chart/openfaas/templates/gateway-dep.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,8 @@ spec:
value: "{{ .Values.faasnetes.livenessProbe.timeoutSeconds }}"
- name: liveness_probe_period_seconds
value: "{{ .Values.faasnetes.livenessProbe.periodSeconds }}"
- name: cluster_role
value: "{{ .Values.clusterRole }}"
ports:
- containerPort: 8081
protocol: TCP
Expand Down
3 changes: 3 additions & 0 deletions chart/openfaas/templates/operator-rbac.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,9 @@ rules:
- apiGroups: [""]
resources: ["pods", "pods/log", "namespaces", "endpoints"]
verbs: ["get", "list", "watch"]
- apiGroups: [""]
resources: ["events"]
verbs: ["get", "list", "watch", "create", "update", "patch", "delete"]
---
apiVersion: rbac.authorization.k8s.io/v1
kind: ClusterRoleBinding
Expand Down
9 changes: 7 additions & 2 deletions main.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,10 +103,15 @@ func main() {
// auto-scaling is does via the HTTP API that acts on the deployment Spec.Replicas
defaultResync := time.Minute * 5

kubeInformerOpt := kubeinformers.WithNamespace(config.DefaultFunctionNamespace)
namespaceScope := config.DefaultFunctionNamespace
if operator && config.ClusterRole {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given that we have shared code now, would it make sense to have that namespaceScope set even when we're not using the operator?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did this as precaution for the feature, otherwise we can merge those, I would need to test the case before doing this though

namespaceScope = ""
}

kubeInformerOpt := kubeinformers.WithNamespace(namespaceScope)
kubeInformerFactory := kubeinformers.NewSharedInformerFactoryWithOptions(kubeClient, defaultResync, kubeInformerOpt)

faasInformerOpt := informers.WithNamespace(config.DefaultFunctionNamespace)
faasInformerOpt := informers.WithNamespace(namespaceScope)
faasInformerFactory := informers.NewSharedInformerFactoryWithOptions(faasClient, defaultResync, faasInformerOpt)

// this is where we need to swap to the faasInformerFactory
Expand Down
4 changes: 4 additions & 0 deletions pkg/config/read_config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ func (ReadConfig) Read(hasEnv ftypes.HasEnv) (BootstrapConfig, error) {

cfg.DefaultFunctionNamespace = ftypes.ParseString(hasEnv.Getenv("function_namespace"), "default")
cfg.ProfilesNamespace = ftypes.ParseString(hasEnv.Getenv("profiles_namespace"), cfg.DefaultFunctionNamespace)
cfg.ClusterRole = ftypes.ParseBoolValue(hasEnv.Getenv("cluster_role"), false)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we want to think about this internal as "cluster role" or something else, like multiple_namespaces, single_namespace?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Internally would make sense to rename the variable for more friendly experience for the developer, on the other hand this code revolves around kubernetes so I believe developers should be aware of what ClusterRole is


cfg.HTTPProbe = httpProbe
cfg.SetNonRootUser = setNonRootUser
Expand Down Expand Up @@ -105,6 +106,8 @@ type BootstrapConfig struct {
ProfilesNamespace string
// FaaSConfig contains the configuration for the FaaSProvider
FaaSConfig ftypes.FaaSConfig
// ClusterRole determines whether the operator should have cluster wide access
ClusterRole bool
}

// Fprint pretty-prints the config with the stdlib logger. One line per config value.
Expand All @@ -128,5 +131,6 @@ func (c BootstrapConfig) Fprint(verbose bool) {
log.Printf("LivenessProbeInitialDelaySeconds: %d\n", c.LivenessProbeInitialDelaySeconds)
log.Printf("LivenessProbeTimeoutSeconds: %d\n", c.LivenessProbeTimeoutSeconds)
log.Printf("LivenessProbePeriodSeconds: %d\n", c.LivenessProbePeriodSeconds)
log.Printf("ClusterRole: %v\n", c.ClusterRole)
}
}
7 changes: 4 additions & 3 deletions pkg/handlers/namespaces.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ func MakeNamespacesLister(defaultNamespace string, clientset kubernetes.Interfac
return func(w http.ResponseWriter, r *http.Request) {
log.Println("Query namespaces")

res := list(defaultNamespace, clientset)
res := ListNamespaces(defaultNamespace, clientset)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did this go to being a public function (capital letter) so that it could be used elsewhere?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I use it in the makeNamespace handler of the operator, though as per Lucas's feedback we don't really need exposing this, we can just reuse the namespace resolver from the faas-netes handlers and just delete pkg/server/namespace.go package as faas-netes and operator resolve namespaces available to the framework the same way


out, _ := json.Marshal(res)
w.Header().Set("Content-Type", "application/json")
Expand Down Expand Up @@ -63,7 +63,7 @@ func NewNamespaceResolver(defaultNamespace string, kube kubernetes.Interface) Na
r.Body = ioutil.NopCloser(bytes.NewBuffer(body))
}

allowedNamespaces := list(defaultNamespace, kube)
allowedNamespaces := ListNamespaces(defaultNamespace, kube)
ok := findNamespace(req.Namespace, allowedNamespaces)
if !ok {
return req.Namespace, fmt.Errorf("unable to manage secrets within the %s namespace", req.Namespace)
Expand All @@ -73,7 +73,8 @@ func NewNamespaceResolver(defaultNamespace string, kube kubernetes.Interface) Na
}
}

func list(defaultNamespace string, clientset kubernetes.Interface) []string {
// ListNamespaces lists all namespaces annotated with openfaas true
func ListNamespaces(defaultNamespace string, clientset kubernetes.Interface) []string {
listOptions := metav1.ListOptions{}
namespaces, err := clientset.CoreV1().Namespaces().List(context.TODO(), listOptions)

Expand Down
35 changes: 18 additions & 17 deletions pkg/handlers/secrets.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,26 +14,27 @@ import (
)

// MakeSecretHandler makes a handler for Create/List/Delete/Update of
//secrets in the Kubernetes API
// secrets in the Kubernetes API
func MakeSecretHandler(defaultNamespace string, kube kubernetes.Interface) http.HandlerFunc {
handler := secretsHandler{
lookupNamespace: NewNamespaceResolver(defaultNamespace, kube),
secrets: k8s.NewSecretsClient(kube),
handler := SecretsHandler{
LookupNamespace: NewNamespaceResolver(defaultNamespace, kube),
Secrets: k8s.NewSecretsClient(kube),
}
return handler.ServeHTTP
}

type secretsHandler struct {
secrets k8s.SecretsClient
lookupNamespace NamespaceResolver
// SecretsHandler enabling to create openfaas secrets across namespaces
type SecretsHandler struct {
Secrets k8s.SecretsClient
LookupNamespace NamespaceResolver
}

func (h secretsHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
func (h SecretsHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
if r.Body != nil {
defer r.Body.Close()
}

lookupNamespace, err := h.lookupNamespace(r)
lookupNamespace, err := h.LookupNamespace(r)
if err != nil {
switch err.Error() {
case "unable to unmarshal Secret request":
Expand Down Expand Up @@ -61,8 +62,8 @@ func (h secretsHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
}
}

func (h secretsHandler) listSecrets(namespace string, w http.ResponseWriter, r *http.Request) {
res, err := h.secrets.List(namespace)
func (h SecretsHandler) listSecrets(namespace string, w http.ResponseWriter, r *http.Request) {
res, err := h.Secrets.List(namespace)
if err != nil {
status, reason := ProcessErrorReasons(err)
log.Printf("Secret list error reason: %s, %v\n", reason, err)
Expand All @@ -88,7 +89,7 @@ func (h secretsHandler) listSecrets(namespace string, w http.ResponseWriter, r *
w.Write(secretsBytes)
}

func (h secretsHandler) createSecret(namespace string, w http.ResponseWriter, r *http.Request) {
func (h SecretsHandler) createSecret(namespace string, w http.ResponseWriter, r *http.Request) {
secret := types.Secret{}
err := json.NewDecoder(r.Body).Decode(&secret)
if err != nil {
Expand All @@ -98,7 +99,7 @@ func (h secretsHandler) createSecret(namespace string, w http.ResponseWriter, r
}

secret.Namespace = namespace
err = h.secrets.Create(secret)
err = h.Secrets.Create(secret)
if err != nil {
status, reason := ProcessErrorReasons(err)
log.Printf("Secret create error reason: %s, %v\n", reason, err)
Expand All @@ -109,7 +110,7 @@ func (h secretsHandler) createSecret(namespace string, w http.ResponseWriter, r
w.WriteHeader(http.StatusAccepted)
}

func (h secretsHandler) replaceSecret(namespace string, w http.ResponseWriter, r *http.Request) {
func (h SecretsHandler) replaceSecret(namespace string, w http.ResponseWriter, r *http.Request) {
secret := types.Secret{}
err := json.NewDecoder(r.Body).Decode(&secret)
if err != nil {
Expand All @@ -119,7 +120,7 @@ func (h secretsHandler) replaceSecret(namespace string, w http.ResponseWriter, r
}

secret.Namespace = namespace
err = h.secrets.Replace(secret)
err = h.Secrets.Replace(secret)
if err != nil {
status, reason := ProcessErrorReasons(err)
log.Printf("Secret update error reason: %s, %v\n", reason, err)
Expand All @@ -130,7 +131,7 @@ func (h secretsHandler) replaceSecret(namespace string, w http.ResponseWriter, r
w.WriteHeader(http.StatusAccepted)
}

func (h secretsHandler) deleteSecret(namespace string, w http.ResponseWriter, r *http.Request) {
func (h SecretsHandler) deleteSecret(namespace string, w http.ResponseWriter, r *http.Request) {
secret := types.Secret{}
err := json.NewDecoder(r.Body).Decode(&secret)
if err != nil {
Expand All @@ -139,7 +140,7 @@ func (h secretsHandler) deleteSecret(namespace string, w http.ResponseWriter, r
return
}

err = h.secrets.Delete(namespace, secret.Name)
err = h.Secrets.Delete(namespace, secret.Name)
if err != nil {
status, reason := ProcessErrorReasons(err)
log.Printf("Secret delete error reason: %s, %v\n", reason, err)
Expand Down
7 changes: 6 additions & 1 deletion pkg/server/apply.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import (
"k8s.io/klog"
)

func makeApplyHandler(namespace string, client clientset.Interface) http.HandlerFunc {
func makeApplyHandler(defaultNamespace string, client clientset.Interface) http.HandlerFunc {
return func(w http.ResponseWriter, r *http.Request) {

if r.Body != nil {
Expand All @@ -33,6 +33,11 @@ func makeApplyHandler(namespace string, client clientset.Interface) http.Handler
}
klog.Infof("Deployment request for: %s\n", req.Service)

namespace := defaultNamespace
if len(req.Namespace) > 0 {
namespace = req.Namespace
}

opts := metav1.GetOptions{}
got, err := client.OpenfaasV1().Functions(namespace).Get(context.TODO(), req.Service, opts)
miss := false
Expand Down
17 changes: 15 additions & 2 deletions pkg/server/delete.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,22 @@ import (
glog "k8s.io/klog"
)

func makeDeleteHandler(namespace string, client clientset.Interface) http.HandlerFunc {
func makeDeleteHandler(defaultNamespace string, client clientset.Interface) http.HandlerFunc {
return func(w http.ResponseWriter, r *http.Request) {

q := r.URL.Query()
namespace := q.Get("namespace")

lookupNamespace := defaultNamespace
if len(namespace) > 0 {
lookupNamespace = namespace
}

if namespace == "kube-system" {
http.Error(w, "unable to list within the kube-system namespace", http.StatusUnauthorized)
return
}

if r.Body != nil {
defer r.Body.Close()
}
Expand All @@ -35,7 +48,7 @@ func makeDeleteHandler(namespace string, client clientset.Interface) http.Handle
return
}

err = client.OpenfaasV1().Functions(namespace).
err = client.OpenfaasV1().Functions(lookupNamespace).
Delete(context.TODO(), request.FunctionName, metav1.DeleteOptions{})
if err != nil {
w.WriteHeader(http.StatusInternalServerError)
Expand Down
24 changes: 19 additions & 5 deletions pkg/server/list.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,19 +13,33 @@ import (
glog "k8s.io/klog"
)

func makeListHandler(namespace string,
func makeListHandler(defaultNamespace string,
client clientset.Interface,
deploymentLister appsv1.DeploymentNamespaceLister) http.HandlerFunc {
deploymentLister appsv1.DeploymentLister) http.HandlerFunc {

return func(w http.ResponseWriter, r *http.Request) {
if r.Body != nil {
defer r.Body.Close()
}

q := r.URL.Query()
namespace := q.Get("namespace")

lookupNamespace := defaultNamespace

if len(namespace) > 0 {
lookupNamespace = namespace
}

if lookupNamespace == "kube-system" {
http.Error(w, "unable to list within the kube-system namespace", http.StatusUnauthorized)
return
}

functions := []types.FunctionStatus{}

opts := metav1.ListOptions{}
res, err := client.OpenfaasV1().Functions(namespace).List(context.TODO(), opts)
res, err := client.OpenfaasV1().Functions(lookupNamespace).List(context.TODO(), opts)
if err != nil {
w.WriteHeader(http.StatusBadRequest)
w.Write([]byte(err.Error()))
Expand All @@ -35,7 +49,7 @@ func makeListHandler(namespace string,

for _, item := range res.Items {

desiredReplicas, availableReplicas, err := getReplicas(item.Spec.Name, namespace, deploymentLister)
desiredReplicas, availableReplicas, err := getReplicas(item.Spec.Name, lookupNamespace, deploymentLister)
if err != nil {
glog.Warningf("Function listing getReplicas error: %v", err)
}
Expand All @@ -47,7 +61,7 @@ func makeListHandler(namespace string,
Image: item.Spec.Image,
Labels: item.Spec.Labels,
Annotations: item.Spec.Annotations,
Namespace: namespace,
Namespace: lookupNamespace,
}

functions = append(functions, function)
Expand Down
13 changes: 8 additions & 5 deletions pkg/server/namespace.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,16 +3,19 @@ package server
import (
"encoding/json"
"net/http"

"github.com/openfaas/faas-netes/pkg/handlers"
"k8s.io/client-go/kubernetes"
)

func makeListNamespaceHandler(defaultNamespace string) func(http.ResponseWriter, *http.Request) {
func makeListNamespaceHandler(defaultNamespace string, clientset kubernetes.Interface) func(http.ResponseWriter, *http.Request) {
return func(w http.ResponseWriter, r *http.Request) {
res := handlers.ListNamespaces(defaultNamespace, clientset)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where did defer r.Body.Close() go ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well this function is literally taken from handlers.MakeNamespacesLister which does not have defer r.Body.Close().

On another note I could have just reuse handlers.MakeNamespacesLister instead of copy pasting, but this followed how the other handlers are implemented for the operator logic and bigger refactoring should take place as per Lucas's comment from the previous review

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add it at the top, for belt-and-braces

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point, will add in both places


defer r.Body.Close()

res, _ := json.Marshal([]string{defaultNamespace})
out, _ := json.Marshal(res)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please catch and at least log the error here

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@martindekov ?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed, even if it's the same elsewhere

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I delayed the response here, I also agree

w.Header().Set("Content-Type", "application/json")

w.WriteHeader(http.StatusOK)
w.Write(res)
w.Write(out)
}
}
Loading