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 support to refer to values from ConfigMap in other namespaces #208

Closed
wants to merge 5 commits into from
Closed

Add support to refer to values from ConfigMap in other namespaces #208

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
a03bb2c
Add support to refer to values from ConfigMap and Secret in other nam…
alex-berger Feb 4, 2021
26cfde9
Only allow cross-namespace sharing of resources if resource owner opt…
alex-berger Feb 5, 2021
2ccdb42
Simplify check logic.
alex-berger Feb 5, 2021
6c1b404
rename cross-namespace sharing annotation to helm.toolkit.fluxcd.io/s…
alex-berger Feb 7, 2021
27f0073
Merge branch 'main' into feature/global-values
alex-berger Feb 7, 2021
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
12 changes: 10 additions & 2 deletions api/v2beta1/reference_types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,13 +50,21 @@ type ValuesReference struct {
// +required
Kind string `json:"kind"`

// Name of the values referent. Should reside in the same namespace as the
// referring resource.
// Name of the values referent.
// +kubebuilder:validation:MinLength=1
// +kubebuilder:validation:MaxLength=253
// +required
Name string `json:"name"`

// Namespace of the values referent. If not present, then the namespace of
// the referring resource will be used. If present, the referred resource
// must have an approriate `helm.toolkit.fluxcd.io/share-with` annotation value,
// which contains the namespace of the referring resource.
// +kubebuilder:validation:MinLength=1
// +kubebuilder:validation:MaxLength=253
// +optional
Namespace string `json:"namespace,omitempty"`

// ValuesKey is the data key where the values.yaml or a specific value can be
// found at. Defaults to 'values.yaml'.
// +optional
Expand Down
12 changes: 10 additions & 2 deletions config/crd/bases/helm.toolkit.fluxcd.io_helmreleases.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -525,8 +525,16 @@ spec:
- ConfigMap
type: string
name:
description: Name of the values referent. Should reside in the
same namespace as the referring resource.
description: Name of the values referent.
maxLength: 253
minLength: 1
type: string
namespace:
description: Namespace of the values referent. If not present,
then the namespace of the referring resource will be used.
If present, the referred resource must have an approriate
`helm.toolkit.fluxcd.io/share-with` annotation value, which
contains the namespace of the referring resource.
maxLength: 253
minLength: 1
type: string
Expand Down
79 changes: 76 additions & 3 deletions controllers/helmrelease_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ package controllers

import (
"context"
"encoding/json"
"errors"
"fmt"
"strings"
Expand All @@ -38,6 +39,7 @@ import (
"k8s.io/client-go/rest"
kuberecorder "k8s.io/client-go/tools/record"
"k8s.io/client-go/tools/reference"
"k8s.io/kubectl/pkg/util/slice"
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/builder"
"sigs.k8s.io/controller-runtime/pkg/client"
Expand Down Expand Up @@ -499,6 +501,57 @@ func (r *HelmReleaseReconciler) getServiceAccountToken(ctx context.Context, hr v
return token, nil
}

const (
// Annotation on resources to explicitly share them with other
// namespaces. The annotation must be either an array of strings, containing the
// names of the namespaces with which the resource is shared or a string with
// the wildcard value "*" to share the resource with all namesapces.
shareWithAnnotation = "helm.toolkit.fluxcd.io/share-with"
)

// Check if a resource is shared with a given namespace, according to the following rules:
//
// 1. If resource is in the same namespace, it is shared by default and can be accessed.
// 2. If resource is in a different namespace and has a "helm.toolkit.fluxcd.io/share-with"
// annotation of type string with the wildcard value "*", then it is shared and
// can be accessed.
// 3. If resource is in a different namespace and has a "helm.toolkit.fluxcd.io/share-with"
// annotation (an array of namespace names) which contains the namespace, it is
// shared and can be accessed.
// 4. If non of the above rules applies, the resource is not shared and cannot be
// accessed.
func isSharedWith(kind string, resource *metav1.ObjectMeta, namespace string) error {
if resource != nil {
if resource.Namespace == namespace {
// Rule 1.
return nil
}
if resource.Annotations != nil {
shareWithString := strings.TrimSpace(resource.Annotations[shareWithAnnotation])
if shareWithString == "*" {
// Rule 2.
return nil
}
if shareWithString != "" {
var shareWith []string = make([]string, 0)
if err := json.Unmarshal([]byte(shareWithString), &shareWith); err != nil {
return fmt.Errorf(
"%s '%s/%s' has invalid %s annotation value, expected string or array of string got '%s': %w",
kind, resource.Namespace, resource.Name, shareWithAnnotation, shareWithString, err)
}
if slice.ContainsString(shareWith, namespace, nil) {
// Rule 3.
return nil
}
}
}
}
// Rule 4.
return fmt.Errorf(
"%s '%s/%s' is not shared with namespace '%s' (change its '%s' annotation to enable sharing)",
kind, resource.Namespace, resource.Name, namespace, shareWithAnnotation)
Comment on lines +524 to +552
Copy link

Choose a reason for hiding this comment

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

Consider exiting early to reduce nested logic.
No need to test a map[string]string for nil before checking if it has a particular key, use ok as demonstrated.
I took out checking for an empty string as that will fail unmarshalling which I felt simplified things, but that could be personal preference.
metav1.ResourceObj keys should use the Getters when available, ie. GetAnnotations()

Suggested change
if resource != nil {
if resource.Namespace == namespace {
// Rule 1.
return nil
}
if resource.Annotations != nil {
shareWithString := strings.TrimSpace(resource.Annotations[shareWithAnnotation])
if shareWithString == "*" {
// Rule 2.
return nil
}
if shareWithString != "" {
var shareWith []string = make([]string, 0)
if err := json.Unmarshal([]byte(shareWithString), &shareWith); err != nil {
return fmt.Errorf(
"%s '%s/%s' has invalid %s annotation value, expected string or array of string got '%s': %w",
kind, resource.Namespace, resource.Name, shareWithAnnotation, shareWithString, err)
}
if slice.ContainsString(shareWith, namespace, nil) {
// Rule 3.
return nil
}
}
}
}
// Rule 4.
return fmt.Errorf(
"%s '%s/%s' is not shared with namespace '%s' (change its '%s' annotation to enable sharing)",
kind, resource.Namespace, resource.Name, namespace, shareWithAnnotation)
if resource == nil {
return fmt.Error("nil resource")
}
shareWithString, ok := resource.GetAnnotations()[shareWithAnnotation]
if !ok {
// Rule 4.
return fmt.Errorf(
"%s '%s/%s' is not shared with namespace '%s' (add an annotation, '%s', to enable sharing)",
kind, resource.Namespace, resource.Name, namespace, shareWithAnnotation)
}
shareWithString = strings.TrimSpace(shareWithString)
if resource.Namespace == namespace {
// Rule 1.
return nil
}
if shareWithString == "*" {
// Rule 2.
return nil
}
var shareWith []string
if err := json.Unmarshal([]byte(shareWithString), &shareWith); err != nil {
return fmt.Errorf(
"%s '%s/%s' has invalid %s annotation value, expected string or array of string got '%s': %w",
kind, resource.Namespace, resource.Name, shareWithAnnotation, shareWithString, err)
}
if !slice.ContainsString(shareWith, namespace, nil) {
// Rule 4.
return fmt.Errorf(
"%s '%s/%s' is not shared with namespace '%s' (change its '%s' annotation to enable sharing)",
kind, resource.Namespace, resource.Name, namespace, shareWithAnnotation)
}
// Rule 3.
return nil

I hope this is helpful.

}

// composeValues attempts to resolve all v2beta1.ValuesReference resources
// and merges them as defined. Referenced resources are only retrieved once
// to ensure a single version is taken into account during the merge.
Expand All @@ -509,7 +562,11 @@ func (r *HelmReleaseReconciler) composeValues(ctx context.Context, hr v2.HelmRel
secrets := make(map[string]*corev1.Secret)

for _, v := range hr.Spec.ValuesFrom {
namespacedName := types.NamespacedName{Namespace: hr.Namespace, Name: v.Name}
namespace := v.Namespace
if namespace == "" {
namespace = hr.Namespace
}
namespacedName := types.NamespacedName{Namespace: namespace, Name: v.Name}
var valuesData []byte

switch v.Kind {
Expand All @@ -531,7 +588,15 @@ func (r *HelmReleaseReconciler) composeValues(ctx context.Context, hr v2.HelmRel
}
return nil, err
}
configMaps[namespacedName.String()] = resource
if err := isSharedWith(resource.Kind, &resource.ObjectMeta, hr.Namespace); err != nil {
if !v.Optional {
return nil, err
}
(logr.FromContext(ctx)).Error(err, "Found optional %s '%s', but %s", v.Kind, namespacedName, err)
resource = nil
} else {
configMaps[namespacedName.String()] = resource
}
}
if resource == nil {
if v.Optional {
Expand Down Expand Up @@ -563,7 +628,15 @@ func (r *HelmReleaseReconciler) composeValues(ctx context.Context, hr v2.HelmRel
}
return nil, err
}
secrets[namespacedName.String()] = resource
if err := isSharedWith(resource.Kind, &resource.ObjectMeta, hr.Namespace); err != nil {
if !v.Optional {
return nil, err
}
(logr.FromContext(ctx)).Error(err, "Found optional %s '%s', but %s", v.Kind, namespacedName, err)
resource = nil
} else {
secrets[namespacedName.String()] = resource
}
}
if resource == nil {
if v.Optional {
Expand Down
18 changes: 16 additions & 2 deletions docs/api/helmrelease.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1877,8 +1877,22 @@ string
</em>
</td>
<td>
<p>Name of the values referent. Should reside in the same namespace as the
referring resource.</p>
<p>Name of the values referent.</p>
</td>
</tr>
<tr>
<td>
<code>namespace</code><br>
<em>
string
</em>
</td>
<td>
<em>(Optional)</em>
<p>Namespace of the values referent. If not present, then the namespace of
the referring resource will be used. If present, the referred resource
must have an approriate <code>helm.toolkit.fluxcd.io/share-with</code> annotation value,
which contains the namespace of the referring resource.</p>
</td>
</tr>
<tr>
Expand Down
20 changes: 15 additions & 5 deletions docs/spec/v2beta1/helmreleases.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -427,13 +427,21 @@ type ValuesReference struct {
// +required
Kind string `json:"kind"`

// Name of the values referent. Should reside in the same namespace as the
// referring resource.
// Name of the values referent.
// +kubebuilder:validation:MinLength=1
// +kubebuilder:validation:MaxLength=253
// +required
Name string `json:"name"`

// Namespace of the values referent. If not present, then the namespace of
// the referring resource will be used. If present, the referred resource
// must have an approriate `helm.toolkit.fluxcd.io/share-with` annotation value,
// which contains the namespace of the referring resource.
// +kubebuilder:validation:MinLength=1
// +kubebuilder:validation:MaxLength=253
// +optional
Namespace string `json:"namespace,omitempty"`

// ValuesKey is the data key where the values.yaml or a specific value can be
// found at. Defaults to 'values.yaml'.
// +optional
Expand Down Expand Up @@ -644,7 +652,8 @@ spec:
name: prod-env-values
valuesKey: values-prod.yaml
- kind: Secret
name: prod-tls-values
name: prod-tls-values
namespace: other-namespace
Comment on lines +655 to +656
Copy link

Choose a reason for hiding this comment

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

accidental tab

Suggested change
name: prod-tls-values
namespace: other-namespace
name: prod-tls-values
namespace: other-namespace

valuesKey: crt
targetPath: tls.crt
optional: true
Expand All @@ -653,8 +662,9 @@ spec:
The definition of the listed keys for items in `spec.valuesFrom` is as follows:

- `kind`: Kind of the values referent (`ConfigMap` or `Secret`).
- `name`: Name of the values referent, in the same namespace as the
`HelmRelease`.
- `name`: Name of the values referent.
- `namespace` _(Optional)_: Namespace of the values referent. Defaults to
the namespace of the `HelmRelease`.
- `valuesKey` _(Optional)_: The data key where the values.yaml or a
specific value can be found. Defaults to `values.yaml` when omitted.
- `targetPath` _(Optional)_: The YAML dot notation path at which the
Expand Down
1 change: 1 addition & 0 deletions go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ require (
k8s.io/apimachinery v0.20.2
k8s.io/cli-runtime v0.20.2
k8s.io/client-go v0.20.2
k8s.io/kubectl v0.20.1
rsc.io/letsencrypt v0.0.3 // indirect
sigs.k8s.io/controller-runtime v0.8.0
sigs.k8s.io/kustomize/api v0.7.2
Expand Down