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

Feat add startup error when running in kube system #1031

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Feat add startup error when running in kube system #1031

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
6 changes: 6 additions & 0 deletions main.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,12 @@ func main() {

config.Fprint(verbose)

// use kubeclient to check the current namespace
namespace, _ := k8s.CurrentNamespace()
if namespace == "kube-system" {
Copy link
Member

Choose a reason for hiding this comment

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

This isn't quite the error we were running into.

You probably can run the openfaas core components in the kube-system namespace.

It was that you can't deploy functions there.

So we already have validation in the deployment handlers, but we could do better by checking the functionNamespace variable (if set), and exiting upon startup

Copy link
Member Author

Choose a reason for hiding this comment

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

i thought this was part of the suggestion #1022 (comment)

but i see that what we also need to do is provide a meaningful api error as well

log.Fatal("You cannot run the OpenFaaS provider in the kube-system namespace, please try another namespace.")
}

deployConfig := k8s.DeploymentConfig{
RuntimeHTTPPort: 8080,
HTTPProbe: config.HTTPProbe,
Expand Down
27 changes: 27 additions & 0 deletions pkg/k8s/namespaces.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
package k8s

import (
"io/ioutil"
"os"
"strings"
)

// CurrentNamespace attempts to return the current namespace from the environment
// or from the service account file. If it cannot find the namespace, it returns
// an empty string. This will be empty when the not running in-cluster.
//
// This implementation is based on the clientcmd.inClusterClientConfig.Namespace method.
// This is not exported and not accessible via other methods, so we have to copy it.
func CurrentNamespace() (namespace string, found bool) {
if ns := os.Getenv("POD_NAMESPACE"); ns != "" {
Copy link
Member

@alexellis alexellis Oct 4, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

That is a pretty nice trick. TIL

Although please see my other *comment (typo)

return ns, true
}

if data, err := ioutil.ReadFile("/var/run/secrets/kubernetes.io/serviceaccount/namespace"); err == nil {
if ns := strings.TrimSpace(string(data)); len(ns) > 0 {
return ns, true
}
}

return "", false
}