Skip to content
This repository has been archived by the owner on Nov 1, 2022. It is now read-only.

Commit

Permalink
Merge pull request #2719 from PaulFarver/feature/helm-multiple-namesp…
Browse files Browse the repository at this point in the history
…aces

Namespace whitelisting in helm chart without clusterRole
  • Loading branch information
stefanprodan authored Jan 9, 2020
2 parents d4f2f58 + 89b8137 commit dadd738
Show file tree
Hide file tree
Showing 7 changed files with 45 additions and 36 deletions.
3 changes: 2 additions & 1 deletion chart/flux/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -202,10 +202,11 @@ The following tables lists the configurable parameters of the Flux chart and the
| `env.secretName` | `` | Name of the secret that contains environment variables which should be defined in the Flux container (using `envFrom`)
| `rbac.create` | `true` | If `true`, create and use RBAC resources
| `rbac.pspEnabled` | `false` | If `true`, create and use a restricted pod security policy for Flux pod(s)
| `allowedNamespaces` | `[]` | Allow flux to manage resources in the specified namespaces. The namespace flux is deployed in will always be included
| `serviceAccount.create` | `true` | If `true`, create a new service account
| `serviceAccount.name` | `flux` | Service account to be used
| `serviceAccount.annotations` | `` | Additional Service Account annotations
| `clusterRole.create` | `true` | If `false`, Flux and the Helm Operator will be restricted to the namespace where they are deployed
| `clusterRole.create` | `true` | If `false`, Flux will be restricted to the namespaces given in `allowedNamespaces` and the namespace where it is deployed
| `service.type` | `ClusterIP` | Service type to be used (exposing the Flux API outside of the cluster is not advised)
| `service.port` | `3030` | Service port to be used
| `sync.state` | `git` | Where to keep sync state; either a tag in the upstream repo (`git`), or as an annotation on the SSH secret (`secret`)
Expand Down
2 changes: 1 addition & 1 deletion chart/flux/templates/deployment.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,7 @@ spec:
{{- end }}
args:
{{- if not .Values.clusterRole.create }}
- --k8s-allow-namespace={{ .Release.Namespace }}
- --k8s-allow-namespace={{ join "," (append .Values.allowedNamespaces .Release.Namespace) }}
{{- end}}
{{- if .Values.logFormat }}
- --log-format={{ .Values.logFormat }}
Expand Down
30 changes: 17 additions & 13 deletions chart/flux/templates/rbac-role.yaml
Original file line number Diff line number Diff line change
@@ -1,13 +1,15 @@
{{- if and .Values.rbac.create (eq .Values.clusterRole.create false) -}}
{{- range $namespace := (append .Values.allowedNamespaces .Release.Namespace) }}
apiVersion: rbac.authorization.k8s.io/v1beta1
kind: Role
metadata:
name: {{ template "flux.fullname" . }}
name: {{ template "flux.fullname" $ }}
namespace: {{ $namespace }}
labels:
app: {{ template "flux.name" . }}
chart: {{ template "flux.chart" . }}
release: {{ .Release.Name }}
heritage: {{ .Release.Service }}
app: {{ template "flux.name" $ }}
chart: {{ template "flux.chart" $ }}
release: {{ $.Release.Name }}
heritage: {{ $.Release.Service }}
rules:
- apiGroups:
- '*'
Expand All @@ -19,21 +21,23 @@ rules:
apiVersion: rbac.authorization.k8s.io/v1beta1
kind: RoleBinding
metadata:
name: {{ template "flux.fullname" . }}
name: {{ template "flux.fullname" $ }}
namespace: {{ $namespace }}
labels:
app: {{ template "flux.name" . }}
chart: {{ template "flux.chart" . }}
release: {{ .Release.Name }}
heritage: {{ .Release.Service }}
app: {{ template "flux.name" $ }}
chart: {{ template "flux.chart" $ }}
release: {{ $.Release.Name }}
heritage: {{ $.Release.Service }}
roleRef:
apiGroup: rbac.authorization.k8s.io
kind: Role
name: {{ template "flux.fullname" . }}
name: {{ template "flux.fullname" $ }}
subjects:
- name: {{ template "flux.serviceAccountName" . }}
namespace: {{ .Release.Namespace | quote }}
- name: {{ template "flux.serviceAccountName" $ }}
namespace: {{ $.Release.Namespace | quote }}
kind: ServiceAccount
---
{{- end }}
apiVersion: rbac.authorization.k8s.io/v1beta1
kind: ClusterRole
metadata:
Expand Down
10 changes: 6 additions & 4 deletions chart/flux/values.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,9 @@ service:
type: ClusterIP
port: 3030

# Specifies which namespaces flux should have access to
allowedNamespaces: []

rbac:
# Specifies whether RBAC resources should be created
create: true
Expand All @@ -32,10 +35,9 @@ serviceAccount:
# Annotations for the Service Account
annotations: {}

# If create is `false` and no name is given, Flux and the Helm
# Operator will be restricted to the namespace where they are
# deployed, and the kubeconfig default context will be set to that
# namespace.
# If create is `false` and no name is given, Flux will be restricted to
# namespaces listed in allowedNamespaces and the namespace where it is
# deployed, and the kubeconfig default context will be set to that namespace.
clusterRole:
create: true
# The name of a cluster role to bind to; if not set and create is
Expand Down
5 changes: 4 additions & 1 deletion cmd/fluxd/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -493,7 +493,10 @@ func main() {

client := kubernetes.MakeClusterClientset(clientset, dynamicClientset, fhrClientset, hrClientset, discoClientset)
kubectlApplier := kubernetes.NewKubectl(kubectl, restClientConfig)
allowedNamespaces := append(*k8sNamespaceWhitelist, *k8sAllowNamespace...)
allowedNamespaces := make(map[string]struct{})
for _, n := range append(*k8sNamespaceWhitelist, *k8sAllowNamespace...) {
allowedNamespaces[n] = struct{}{}
}
k8sInst := kubernetes.NewCluster(client, kubectlApplier, sshKeyRing, logger, allowedNamespaces, *registryExcludeImage)
k8sInst.GC = *syncGC
k8sInst.DryGC = *dryGC
Expand Down
14 changes: 5 additions & 9 deletions pkg/cluster/kubernetes/kubernetes.go
Original file line number Diff line number Diff line change
Expand Up @@ -106,15 +106,15 @@ type Cluster struct {
syncErrors map[resource.ID]error
muSyncErrors sync.RWMutex

allowedNamespaces []string
allowedNamespaces map[string]struct{}
loggedAllowedNS map[string]bool // to keep track of whether we've logged a problem with seeing an allowed namespace

imageExcludeList []string
mu sync.Mutex
}

// NewCluster returns a usable cluster.
func NewCluster(client ExtendedClient, applier Applier, sshKeyRing ssh.KeyRing, logger log.Logger, allowedNamespaces []string, imageExcludeList []string) *Cluster {
func NewCluster(client ExtendedClient, applier Applier, sshKeyRing ssh.KeyRing, logger log.Logger, allowedNamespaces map[string]struct{}, imageExcludeList []string) *Cluster {
c := &Cluster{
client: client,
applier: applier,
Expand Down Expand Up @@ -304,7 +304,7 @@ func (c *Cluster) PublicSSHKey(regenerate bool) (ssh.PublicKey, error) {
func (c *Cluster) getAllowedAndExistingNamespaces(ctx context.Context) ([]string, error) {
if len(c.allowedNamespaces) > 0 {
nsList := []string{}
for _, name := range c.allowedNamespaces {
for name, _ := range c.allowedNamespaces {
if err := ctx.Err(); err != nil {
return nil, err
}
Expand Down Expand Up @@ -350,12 +350,8 @@ func (c *Cluster) IsAllowedResource(id resource.ID) bool {
namespaceToCheck = name
}

for _, allowedNS := range c.allowedNamespaces {
if namespaceToCheck == allowedNS {
return true
}
}
return false
_, ok := c.allowedNamespaces[namespaceToCheck]
return ok
}

type yamlThroughJSON struct {
Expand Down
17 changes: 10 additions & 7 deletions pkg/cluster/kubernetes/kubernetes_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package kubernetes
import (
"context"
"reflect"
"sort"
"testing"

"github.com/go-kit/kit/log"
Expand All @@ -27,20 +28,22 @@ func testGetAllowedNamespaces(t *testing.T, namespace []string, expected []strin
clientset := fakekubernetes.NewSimpleClientset(newNamespace("default"),
newNamespace("kube-system"))
client := ExtendedClient{coreClient: clientset}
c := NewCluster(client, nil, nil, log.NewNopLogger(), namespace, []string{})
allowedNamespaces := make(map[string]struct{})
for _, n := range namespace {
allowedNamespaces[n] = struct{}{}
}
c := NewCluster(client, nil, nil, log.NewNopLogger(), allowedNamespaces, []string{})

namespaces, err := c.getAllowedAndExistingNamespaces(context.Background())
if err != nil {
t.Errorf("The error should be nil, not: %s", err)
}

result := []string{}
for _, namespace := range namespaces {
result = append(result, namespace)
}
sort.Strings(namespaces) // We cannot be sure of the namespace order, since they are saved in a map in cluster struct
sort.Strings(expected)

if reflect.DeepEqual(result, expected) != true {
t.Errorf("Unexpected namespaces: %v != %v.", result, expected)
if reflect.DeepEqual(namespaces, expected) != true {
t.Errorf("Unexpected namespaces: %v != %v.", namespaces, expected)
}
}

Expand Down

0 comments on commit dadd738

Please sign in to comment.