Skip to content

Commit eb22f48

Browse files
bentitobentito
committed
Redo config merging a bit and test thereof
Signed-off-by: Brett Tofel <[email protected]>
1 parent f47e271 commit eb22f48

File tree

3 files changed

+128
-108
lines changed

3 files changed

+128
-108
lines changed

‎api/v1/clusterextensionconfig_deepcopy.go

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
package v1
2+
3+
import (
4+
corev1 "k8s.io/api/core/v1"
5+
apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
6+
)
7+
8+
// DeepCopyInto copies the receiver into out. in must be non-nil.
9+
func (in *ClusterExtensionConfig) DeepCopyInto(out *ClusterExtensionConfig) {
10+
*out = *in
11+
if in.Inline != nil {
12+
out.Inline = make(map[string]apiextensionsv1.JSON, len(in.Inline))
13+
for k, v := range in.Inline {
14+
if v.Raw != nil {
15+
out.Inline[k] = apiextensionsv1.JSON{Raw: append([]byte(nil), v.Raw...)}
16+
} else {
17+
out.Inline[k] = apiextensionsv1.JSON{}
18+
}
19+
}
20+
}
21+
if in.SecretRef != nil {
22+
out.SecretRef = new(corev1.SecretKeySelector)
23+
in.SecretRef.DeepCopyInto(out.SecretRef)
24+
}
25+
}
26+
27+
// DeepCopy creates a new ClusterExtensionConfig by copying the receiver.
28+
func (in *ClusterExtensionConfig) DeepCopy() *ClusterExtensionConfig {
29+
if in == nil {
30+
return nil
31+
}
32+
out := new(ClusterExtensionConfig)
33+
in.DeepCopyInto(out)
34+
return out
35+
}

‎internal/operator-controller/applier/helm.go

Lines changed: 49 additions & 68 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@ import (
2020
corev1 "k8s.io/api/core/v1"
2121
rbacv1 "k8s.io/api/rbac/v1"
2222
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
23-
apitypes "k8s.io/apimachinery/pkg/types"
2423
apimachyaml "k8s.io/apimachinery/pkg/util/yaml"
2524
"sigs.k8s.io/controller-runtime/pkg/client"
2625
"sigs.k8s.io/controller-runtime/pkg/log"
@@ -72,33 +71,6 @@ type Helm struct {
7271
Client client.Client
7372
}
7473

75-
func decodeValues(data []byte) (map[string]any, error) {
76-
m := map[string]any{}
77-
if len(bytes.TrimSpace(data)) == 0 {
78-
return m, nil
79-
}
80-
j, err := apimachyaml.ToJSON(data)
81-
if err != nil {
82-
return nil, err
83-
}
84-
if err := json.Unmarshal(j, &m); err != nil {
85-
return nil, err
86-
}
87-
return m, nil
88-
}
89-
90-
func mergeValueMaps(dst, src map[string]any) {
91-
for k, v := range src {
92-
if dstMap, ok := dst[k].(map[string]any); ok {
93-
if srcMap, ok2 := v.(map[string]any); ok2 {
94-
mergeValueMaps(dstMap, srcMap)
95-
continue
96-
}
97-
}
98-
dst[k] = v
99-
}
100-
}
101-
10274
// shouldSkipPreflight is a helper to determine if the preflight check is CRDUpgradeSafety AND
10375
// if it is set to enforcement None.
10476
func shouldSkipPreflight(ctx context.Context, preflight Preflight, ext *ocv1.ClusterExtension, state string) bool {
@@ -153,41 +125,55 @@ func (h *Helm) runPreAuthorizationChecks(ctx context.Context, ext *ocv1.ClusterE
153125
return nil
154126
}
155127

156-
func (h *Helm) Apply(ctx context.Context, contentFS fs.FS, ext *ocv1.ClusterExtension, objectLabels map[string]string, storageLabels map[string]string) ([]client.Object, string, error) {
157-
chrt, err := h.buildHelmChart(contentFS, ext)
158-
if err != nil {
159-
return nil, "", err
128+
func (h *Helm) configValues(ctx context.Context, ext *ocv1.ClusterExtension) (map[string]interface{}, error) {
129+
cfg := map[string]interface{}{}
130+
if ext.Spec.Config == nil {
131+
return cfg, nil
160132
}
161-
// gather configuration values from the ClusterExtension specification
162-
// and any referenced Secrets, merging them into a single map
163-
valuesMap := map[string]any{}
164-
if ext.Spec.Config != nil {
165-
if ext.Spec.Config.Inline != nil && len(ext.Spec.Config.Inline.Raw) > 0 {
166-
inlineMap, err := decodeValues(ext.Spec.Config.Inline.Raw)
167-
if err != nil {
168-
return nil, "", fmt.Errorf("invalid spec.config.inline: %w", err)
169-
}
170-
mergeValueMaps(valuesMap, inlineMap)
171-
}
172-
if ext.Spec.Config.SecretRef != nil {
173-
if h.Client == nil {
174-
return nil, "", errors.New("client is nil")
175-
}
176-
secret := &corev1.Secret{}
177-
nn := apitypes.NamespacedName{Name: ext.Spec.Config.SecretRef.Name, Namespace: ext.Spec.Namespace}
178-
if err := h.Client.Get(ctx, nn, secret); err != nil {
179-
return nil, "", fmt.Errorf("failed to get config secret: %w", err)
180-
}
181-
data, ok := secret.Data[ext.Spec.Config.SecretRef.Key]
182-
if !ok {
183-
return nil, "", fmt.Errorf("secret %q missing key %q", nn.Name, ext.Spec.Config.SecretRef.Key)
133+
if ext.Spec.Config.Inline != nil {
134+
for k, v := range ext.Spec.Config.Inline {
135+
if len(v.Raw) == 0 {
136+
cfg[k] = nil
137+
continue
184138
}
185-
secretMap, err := decodeValues(data)
186-
if err != nil {
187-
return nil, "", fmt.Errorf("invalid config secret %q key %q: %w", nn.Name, ext.Spec.Config.SecretRef.Key, err)
139+
var val interface{}
140+
if err := json.Unmarshal(v.Raw, &val); err != nil {
141+
return nil, fmt.Errorf("invalid JSON in spec.config.inline[%s]: %w", k, err)
188142
}
189-
mergeValueMaps(valuesMap, secretMap)
143+
cfg[k] = val
144+
}
145+
}
146+
if ext.Spec.Config.SecretRef != nil {
147+
if h.Client == nil {
148+
return nil, fmt.Errorf("secretRef specified but Helm client is nil")
149+
}
150+
secret := &corev1.Secret{}
151+
if err := h.Client.Get(ctx, client.ObjectKey{Name: ext.Spec.Config.SecretRef.Name, Namespace: ext.Spec.Namespace}, secret); err != nil {
152+
return nil, fmt.Errorf("failed to get config secret: %w", err)
190153
}
154+
data, ok := secret.Data[ext.Spec.Config.SecretRef.Key]
155+
if !ok {
156+
return nil, fmt.Errorf("missing key %s in secret %s", ext.Spec.Config.SecretRef.Key, ext.Spec.Config.SecretRef.Name)
157+
}
158+
var secretCfg map[string]interface{}
159+
if err := json.Unmarshal(data, &secretCfg); err != nil {
160+
return nil, fmt.Errorf("invalid JSON in secret %s/%s: %w", ext.Spec.Namespace, ext.Spec.Config.SecretRef.Name, err)
161+
}
162+
for k, v := range secretCfg {
163+
cfg[k] = v
164+
}
165+
}
166+
return cfg, nil
167+
}
168+
169+
func (h *Helm) Apply(ctx context.Context, contentFS fs.FS, ext *ocv1.ClusterExtension, objectLabels map[string]string, storageLabels map[string]string) ([]client.Object, string, error) {
170+
chrt, err := h.buildHelmChart(ctx, contentFS, ext)
171+
if err != nil {
172+
return nil, "", err
173+
}
174+
valuesMap, err := h.configValues(ctx, ext)
175+
if err != nil {
176+
return nil, "", err
191177
}
192178
values := chartutil.Values{"Values": valuesMap}
193179

@@ -266,7 +252,7 @@ func (h *Helm) Apply(ctx context.Context, contentFS fs.FS, ext *ocv1.ClusterExte
266252
return relObjects, state, nil
267253
}
268254

269-
func (h *Helm) buildHelmChart(bundleFS fs.FS, ext *ocv1.ClusterExtension) (*chart.Chart, error) {
255+
func (h *Helm) buildHelmChart(ctx context.Context, bundleFS fs.FS, ext *ocv1.ClusterExtension) (*chart.Chart, error) {
270256
if h.BundleToHelmChartConverter == nil {
271257
return nil, errors.New("BundleToHelmChartConverter is nil")
272258
}
@@ -285,14 +271,9 @@ func (h *Helm) buildHelmChart(bundleFS fs.FS, ext *ocv1.ClusterExtension) (*char
285271
}
286272
}
287273

288-
// Unmarshal any provided configuration on the ClusterExtension and pass it
289-
// to the registry+v1 renderer so that static manifests (e.g. ConfigMaps)
290-
// can be overridden with values from spec.config.
291-
cfg := map[string]interface{}{}
292-
if ext.Spec.Config != nil && ext.Spec.Config.Raw != nil {
293-
if err := json.Unmarshal(ext.Spec.Config.Raw, &cfg); err != nil {
294-
return nil, fmt.Errorf("invalid JSON in spec.config: %w", err)
295-
}
274+
cfg, err := h.configValues(ctx, ext)
275+
if err != nil {
276+
return nil, err
296277
}
297278

298279
return h.BundleToHelmChartConverter.ToHelmChartWithConfig(source.FromFS(bundleFS), ext.Spec.Namespace, watchNamespace, cfg)

‎internal/operator-controller/applier/helm_test.go

Lines changed: 44 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ import (
2020
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
2121
featuregatetesting "k8s.io/component-base/featuregate/testing"
2222
"sigs.k8s.io/controller-runtime/pkg/client"
23-
clientfake "sigs.k8s.io/controller-runtime/pkg/client/fake"
23+
fakeclient "sigs.k8s.io/controller-runtime/pkg/client/fake"
2424

2525
helmclient "github.com/operator-framework/helm-operator-plugins/pkg/client"
2626

@@ -228,6 +228,49 @@ func TestApply_Base(t *testing.T) {
228228
})
229229
}
230230

231+
func TestApply_ConfigMerging(t *testing.T) {
232+
secret := &corev1.Secret{
233+
ObjectMeta: metav1.ObjectMeta{Name: "cfg", Namespace: "ns"},
234+
Data: map[string][]byte{
235+
"config": []byte(`{"secretKey":"secretValue","sharedKey":"secretVal"}`),
236+
},
237+
}
238+
cl := fakeclient.NewClientBuilder().WithObjects(secret).Build()
239+
240+
ext := &ocv1.ClusterExtension{
241+
ObjectMeta: metav1.ObjectMeta{Name: "test", Namespace: "ns"},
242+
Spec: ocv1.ClusterExtensionSpec{
243+
Namespace: "ns",
244+
ServiceAccount: ocv1.ServiceAccountReference{Name: "sa"},
245+
Source: ocv1.SourceConfig{SourceType: ocv1.SourceTypeCatalog, Catalog: &ocv1.CatalogFilter{PackageName: "pkg"}},
246+
Config: &ocv1.ClusterExtensionConfig{
247+
Inline: map[string]apiextensionsv1.JSON{
248+
"inlineKey": {Raw: []byte(`"inlineValue"`)},
249+
"sharedKey": {Raw: []byte(`"inlineVal"`)},
250+
},
251+
SecretRef: &corev1.SecretKeySelector{LocalObjectReference: corev1.LocalObjectReference{Name: "cfg"}, Key: "config"},
252+
},
253+
},
254+
}
255+
256+
mag := &mockActionGetter{desiredRel: &release.Release{Manifest: validManifest}}
257+
258+
helmApplier := applier.Helm{
259+
ActionClientGetter: mag,
260+
BundleToHelmChartConverter: &fakeBundleToHelmChartConverter{fn: func(bundle source.BundleSource, installNamespace string, watchNamespace string) (*chart.Chart, error) {
261+
return &chart.Chart{}, nil
262+
}},
263+
Client: cl,
264+
}
265+
266+
_, _, err := helmApplier.Apply(context.TODO(), validFS, ext, testObjectLabels, testStorageLabels)
267+
require.NoError(t, err)
268+
require.NotNil(t, mag.vals)
269+
assert.Equal(t, "inlineValue", mag.vals["inlineKey"])
270+
assert.Equal(t, "secretValue", mag.vals["secretKey"])
271+
assert.Equal(t, "secretVal", mag.vals["sharedKey"])
272+
}
273+
231274
func TestApply_Installation(t *testing.T) {
232275
t.Run("fails during dry-run installation", func(t *testing.T) {
233276
mockAcg := &mockActionGetter{
@@ -626,45 +669,6 @@ func TestApply_RegistryV1ToChartConverterIntegration(t *testing.T) {
626669
})
627670
}
628671

629-
func TestApply_ConfigMerging(t *testing.T) {
630-
secret := &corev1.Secret{
631-
ObjectMeta: metav1.ObjectMeta{Name: "cfg", Namespace: "ns"},
632-
Data: map[string][]byte{"values": []byte("foo:\n baz: 2\n")},
633-
}
634-
fakeClient := clientfake.NewClientBuilder().WithObjects(secret).Build()
635-
636-
mag := &mockActionGetter{
637-
getClientErr: driver.ErrReleaseNotFound,
638-
desiredRel: &release.Release{Manifest: "---\n"},
639-
}
640-
helmApplier := applier.Helm{
641-
ActionClientGetter: mag,
642-
BundleToHelmChartConverter: &fakeBundleToHelmChartConverter{fn: func(bundle source.BundleSource, installNamespace string, watchNamespace string) (*chart.Chart, error) {
643-
return &chart.Chart{}, nil
644-
}},
645-
Client: fakeClient,
646-
}
647-
648-
ext := &ocv1.ClusterExtension{
649-
ObjectMeta: metav1.ObjectMeta{Name: "test"},
650-
Spec: ocv1.ClusterExtensionSpec{
651-
Namespace: "ns",
652-
ServiceAccount: ocv1.ServiceAccountReference{Name: "sa"},
653-
Source: ocv1.SourceConfig{SourceType: ocv1.SourceTypeCatalog, Catalog: &ocv1.CatalogFilter{PackageName: "pkg"}},
654-
Config: &ocv1.ClusterExtensionConfig{
655-
Inline: &apiextensionsv1.JSON{Raw: []byte("foo:\n bar: 1\n")},
656-
SecretRef: &corev1.SecretKeySelector{LocalObjectReference: corev1.LocalObjectReference{Name: "cfg"}, Key: "values"},
657-
},
658-
},
659-
}
660-
661-
_, _, err := helmApplier.Apply(context.TODO(), validFS, ext, testObjectLabels, testStorageLabels)
662-
require.NoError(t, err)
663-
664-
expected := map[string]any{"foo": map[string]any{"bar": float64(1), "baz": float64(2)}}
665-
assert.Equal(t, expected, mag.vals["Values"])
666-
}
667-
668672
type fakeBundleToHelmChartConverter struct {
669673
fn func(source.BundleSource, string, string) (*chart.Chart, error)
670674
}

0 commit comments

Comments
 (0)