Skip to content

Commit e15577b

Browse files
dee0sapdee0sap
authored
Resolve 766. Configuration can optionally handle missing ConfigMap (#779)
<!-- markdownlint-disable MD041 --> #### What this PR does / why we need it See [issue 766](open-component-model/ocm-project#766) #### Which issue(s) this PR is related to See [issue 766](open-component-model/ocm-project#766) --------- Signed-off-by: [email protected] <[email protected]>
1 parent 5c684a0 commit e15577b

File tree

5 files changed

+133
-38
lines changed

5 files changed

+133
-38
lines changed

‎api/v1alpha1/mutation_types.go‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,11 @@ type ConfigMapSource struct {
5959
Key string `json:"key"`
6060
// +optional
6161
SubPath string `json:"subPath,omitempty"`
62+
// Optional marks this ConfigMapSource as optional. When set, a not found
63+
// error for the configmap reference is ignored, but any Key, Subpath or
64+
// transient error will still result in a reconciliation failure.
65+
// +optional
66+
Optional bool `json:"optional,omitempty"`
6267
}
6368

6469
type FluxValuesSource struct {

‎controllers/configuration_controller_test.go‎

Lines changed: 96 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ import (
2222
"github.com/stretchr/testify/require"
2323
appsv1 "k8s.io/api/apps/v1"
2424
corev1 "k8s.io/api/core/v1"
25+
k8sapierr "k8s.io/apimachinery/pkg/api/errors"
2526
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
2627
"k8s.io/apimachinery/pkg/types"
2728
"k8s.io/client-go/tools/record"
@@ -852,13 +853,18 @@ configuration:
852853
}
853854

854855
func TestConfigurationValuesFrom(t *testing.T) {
856+
commonExpectedData := map[string]string{"PODINFO_UI_MESSAGE": "this is a new message", "PODINFO_UI_COLOR": "bittersweet"}
857+
855858
testCases := []struct {
856-
name string
857-
configuration func(source client.Object) *v1alpha1.Configuration
858-
setup func() client.Object
859+
name string
860+
expectedError error
861+
expectedConfig map[string]string
862+
configuration func(source client.Object) *v1alpha1.Configuration
863+
setup func() client.Object
859864
}{
860865
{
861-
name: "configuration values from GitRepository",
866+
name: "configuration values from GitRepository",
867+
expectedConfig: commonExpectedData,
862868
configuration: func(source client.Object) *v1alpha1.Configuration {
863869
configuration := DefaultConfiguration.DeepCopy()
864870
configuration.Status.SnapshotName = "configuration-snapshot"
@@ -889,7 +895,8 @@ func TestConfigurationValuesFrom(t *testing.T) {
889895
},
890896
},
891897
{
892-
name: "configuration values from ConfigMap",
898+
name: "configuration values from ConfigMap",
899+
expectedConfig: commonExpectedData,
893900
configuration: func(source client.Object) *v1alpha1.Configuration {
894901
configuration := DefaultConfiguration.DeepCopy()
895902
configuration.Status.SnapshotName = "configuration-snapshot"
@@ -922,6 +929,57 @@ func TestConfigurationValuesFrom(t *testing.T) {
922929
return configMap
923930
},
924931
},
932+
{
933+
name: "configuration values from optional missing ConfigMap",
934+
expectedConfig: map[string]string{"PODINFO_UI_MESSAGE": "Hello, world!", "PODINFO_UI_COLOR": "red"},
935+
configuration: func(client.Object) *v1alpha1.Configuration {
936+
configuration := DefaultConfiguration.DeepCopy()
937+
configuration.Status.SnapshotName = "configuration-snapshot"
938+
configuration.Spec.ValuesFrom = &v1alpha1.ValuesSource{
939+
ConfigMapSource: &v1alpha1.ConfigMapSource{
940+
SourceRef: meta.LocalObjectReference{
941+
Name: "test-config-data-does-not-exist",
942+
},
943+
Key: "values.yaml",
944+
SubPath: "test.backend",
945+
Optional: true,
946+
},
947+
}
948+
configuration.Spec.Values = nil
949+
950+
return configuration
951+
},
952+
setup: func() client.Object {
953+
return nil
954+
},
955+
},
956+
{
957+
name: "configuration values from missing ConfigMap",
958+
expectedError: &k8sapierr.StatusError{
959+
ErrStatus: metav1.Status{
960+
Reason: metav1.StatusReasonNotFound,
961+
},
962+
},
963+
configuration: func(client.Object) *v1alpha1.Configuration {
964+
configuration := DefaultConfiguration.DeepCopy()
965+
configuration.Status.SnapshotName = "configuration-snapshot"
966+
configuration.Spec.ValuesFrom = &v1alpha1.ValuesSource{
967+
ConfigMapSource: &v1alpha1.ConfigMapSource{
968+
SourceRef: meta.LocalObjectReference{
969+
Name: "test-config-data-does-not-exist",
970+
},
971+
Key: "values.yaml",
972+
SubPath: "test.backend",
973+
},
974+
}
975+
configuration.Spec.Values = nil
976+
977+
return configuration
978+
},
979+
setup: func() client.Object {
980+
return nil
981+
},
982+
},
925983
}
926984

927985
for _, tc := range testCases {
@@ -960,8 +1018,12 @@ func TestConfigurationValuesFrom(t *testing.T) {
9601018
},
9611019
}
9621020

1021+
objs := []client.Object{cv, cd, resource}
9631022
configSource := tc.setup()
964-
objs := []client.Object{cv, cd, resource, configSource}
1023+
if configSource != nil {
1024+
objs = append(objs, configSource)
1025+
1026+
}
9651027

9661028
configuration := tc.configuration(configSource)
9671029
configuration.Spec.SourceRef = source
@@ -1017,35 +1079,39 @@ func TestConfigurationValuesFrom(t *testing.T) {
10171079
}, snapshotOutput)
10181080

10191081
t.Log("check if target snapshot has been created and cache was called")
1020-
require.NoError(t, err)
1021-
1022-
t.Log("extracting the passed in data and checking if the configuration worked")
1023-
args := cache.PushDataCallingArgumentsOnCall(0)
1024-
assert.Equal(t, "sha-13092443426051895747", args.Name)
1025-
assert.Equal(t, "1.0.0", args.Version)
1026-
sourceFile := extractFileFromTarGz(t, io.NopCloser(bytes.NewBuffer([]byte(args.Content))), "configmap.yaml")
1027-
configMap := corev1.ConfigMap{}
1028-
assert.NoError(t, yaml.Unmarshal(sourceFile, &configMap))
1029-
assert.Equal(t, map[string]string{"PODINFO_UI_MESSAGE": "this is a new message", "PODINFO_UI_COLOR": "bittersweet"}, configMap.Data)
1030-
1031-
err = client.Get(context.Background(), types.NamespacedName{
1032-
Namespace: configuration.Namespace,
1033-
Name: configuration.Name,
1034-
}, configuration)
1035-
require.NoError(t, err)
1036-
1037-
assert.True(t, conditions.IsTrue(configuration, meta.ReadyCondition))
1038-
10391082
close(recorder.Events)
10401083
event := ""
1041-
for e := range recorder.Events {
1042-
if strings.Contains(e, "Reconciliation finished, next run in") {
1043-
event = e
1084+
if tc.expectedError == nil {
1085+
require.NoError(t, err)
1086+
t.Log("extracting the passed in data and checking if the configuration worked")
1087+
args := cache.PushDataCallingArgumentsOnCall(0)
1088+
assert.Equal(t, "sha-13092443426051895747", args.Name)
1089+
assert.Equal(t, "1.0.0", args.Version)
1090+
sourceFile := extractFileFromTarGz(t, io.NopCloser(bytes.NewBuffer([]byte(args.Content))), "configmap.yaml")
1091+
configMap := corev1.ConfigMap{}
1092+
assert.NoError(t, yaml.Unmarshal(sourceFile, &configMap))
1093+
assert.Equal(t, tc.expectedConfig, configMap.Data)
1094+
1095+
err = client.Get(context.Background(), types.NamespacedName{
1096+
Namespace: configuration.Namespace,
1097+
Name: configuration.Name,
1098+
}, configuration)
1099+
require.NoError(t, err)
10441100

1045-
break
1101+
assert.True(t, conditions.IsTrue(configuration, meta.ReadyCondition))
1102+
1103+
for e := range recorder.Events {
1104+
if strings.Contains(e, "Reconciliation finished, next run in") {
1105+
event = e
1106+
1107+
break
1108+
}
10461109
}
1110+
assert.Contains(t, event, "Reconciliation finished, next run in")
1111+
} else {
1112+
assert.Equal(t, k8sapierr.ReasonForError(tc.expectedError), k8sapierr.ReasonForError(err))
10471113
}
1048-
assert.Contains(t, event, "Reconciliation finished, next run in")
1114+
10491115
})
10501116
}
10511117
}

‎controllers/mutation_reconcile_looper.go‎

Lines changed: 20 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -140,9 +140,9 @@ func (m *MutationReconcileLooper) configure(
140140
ctx context.Context,
141141
data, configObj []byte,
142142
mutationSpec *v1alpha1.MutationSpec,
143-
namespace string,
143+
namespace, name string,
144144
) (string, error) {
145-
configValues, err := m.getValues(ctx, mutationSpec, namespace)
145+
configValues, err := m.getValues(ctx, mutationSpec, namespace, name)
146146
if err != nil {
147147
return "", fmt.Errorf("failed to get values: %w", err)
148148
}
@@ -868,7 +868,9 @@ func (m *MutationReconcileLooper) getComponentVersion(ctx context.Context, obj *
868868

869869
// getValues returns values that can be used for the configuration
870870
// currently it only possible to use inline values OR values from an external source.
871-
func (m *MutationReconcileLooper) getValues(ctx context.Context, obj *v1alpha1.MutationSpec, namespace string) (*apiextensionsv1.JSON, error) {
871+
func (m *MutationReconcileLooper) getValues(
872+
ctx context.Context, obj *v1alpha1.MutationSpec, namespace, name string,
873+
) (*apiextensionsv1.JSON, error) {
872874
if obj.Values != nil {
873875
return obj.Values, nil
874876
}
@@ -882,7 +884,7 @@ func (m *MutationReconcileLooper) getValues(ctx context.Context, obj *v1alpha1.M
882884
}
883885
data = content
884886
case obj.ValuesFrom.ConfigMapSource != nil:
885-
content, err := m.fromConfigMapSource(ctx, obj, namespace)
887+
content, err := m.fromConfigMapSource(ctx, obj, namespace, name)
886888
if err != nil {
887889
return nil, fmt.Errorf("failed to get values from configmap source: %w", err)
888890
}
@@ -912,14 +914,24 @@ func (m *MutationReconcileLooper) getValues(ctx context.Context, obj *v1alpha1.M
912914
return nil, errors.New("no values found")
913915
}
914916

915-
func (m *MutationReconcileLooper) fromConfigMapSource(ctx context.Context, obj *v1alpha1.MutationSpec, namespace string) (map[string]any, error) {
917+
func (m *MutationReconcileLooper) fromConfigMapSource(
918+
ctx context.Context,
919+
obj *v1alpha1.MutationSpec,
920+
namespace, name string,
921+
) (map[string]any, error) {
916922
data := make(map[string]any)
917923
cm := &corev1.ConfigMap{}
918924
key := types.NamespacedName{
919925
Name: obj.ValuesFrom.ConfigMapSource.SourceRef.Name,
920926
Namespace: namespace,
921927
}
922928
if err := m.Client.Get(ctx, key, cm); err != nil {
929+
if obj.ValuesFrom.ConfigMapSource.Optional && apierrors.IsNotFound(err) {
930+
log.FromContext(ctx).Info("optional configmap not found for Configuration", "namespace", namespace, "configuration", name, "configmap", key.Name)
931+
932+
return data, nil
933+
}
934+
923935
return nil, fmt.Errorf("failed to get configmap: %w", err)
924936
}
925937

@@ -996,11 +1008,11 @@ func (m *MutationReconcileLooper) mutate(
9961008
ctx context.Context,
9971009
mutationSpec *v1alpha1.MutationSpec,
9981010
sourceData, configData []byte,
999-
namespace string,
1011+
namespace, name string,
10001012
) (string, error) {
10011013
// if values are not nil then this is configuration
10021014
if mutationSpec.Values != nil || mutationSpec.ValuesFrom != nil {
1003-
sourceDir, err := m.configure(ctx, sourceData, configData, mutationSpec, namespace)
1015+
sourceDir, err := m.configure(ctx, sourceData, configData, mutationSpec, namespace, name)
10041016
if err != nil {
10051017
return "", fmt.Errorf("failed to configure resource: %w", err)
10061018
}
@@ -1042,7 +1054,7 @@ func (m *MutationReconcileLooper) mutateConfigRef(
10421054

10431055
obj.GetStatus().LatestConfigVersion = snapshotID[v1alpha1.ComponentVersionKey]
10441056

1045-
sourceDir, err := m.mutate(ctx, spec, sourceData, configData, obj.GetNamespace())
1057+
sourceDir, err := m.mutate(ctx, spec, sourceData, configData, obj.GetNamespace(), obj.GetName())
10461058
if err != nil {
10471059
return "", ocmmetav1.Identity{}, err
10481060
}

‎deploy/crds/delivery.ocm.software_configurations.yaml‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -303,6 +303,12 @@ spec:
303303
properties:
304304
key:
305305
type: string
306+
optional:
307+
description: |-
308+
Optional marks this ConfigMapSource as optional. When set, a not found
309+
error for the configmap reference is ignored, but any Key, Subpath or
310+
transient error will still result in a reconciliation failure.
311+
type: boolean
306312
sourceRef:
307313
description: LocalObjectReference contains enough information
308314
to locate the referenced Kubernetes resource object.

‎deploy/crds/delivery.ocm.software_localizations.yaml‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -303,6 +303,12 @@ spec:
303303
properties:
304304
key:
305305
type: string
306+
optional:
307+
description: |-
308+
Optional marks this ConfigMapSource as optional. When set, a not found
309+
error for the configmap reference is ignored, but any Key, Subpath or
310+
transient error will still result in a reconciliation failure.
311+
type: boolean
306312
sourceRef:
307313
description: LocalObjectReference contains enough information
308314
to locate the referenced Kubernetes resource object.

0 commit comments

Comments
 (0)