Skip to content

Commit 1ac8e14

Browse files
luksaluksa
authored
Set global.platform when not set by the user (openshift-service-mesh#451)
This change ensures that the `openshift` platform is set correctly even when the `openshift` profile isn't selected via `spec.profile`. Users can now select a profile (e.g. `remote`) and still have the correct platform selected automatically. Signed-off-by: Marko Lukša <[email protected]>
1 parent 16c3e03 commit 1ac8e14

File tree

14 files changed

+136
-30
lines changed

14 files changed

+136
-30
lines changed

‎cmd/main.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -137,14 +137,14 @@ func main() {
137137
defaultProfile = "default"
138138
}
139139

140-
err = istio.NewReconciler(mgr.GetClient(), mgr.GetScheme(), resourceDirectory, defaultProfile).
140+
err = istio.NewReconciler(mgr.GetClient(), mgr.GetScheme(), resourceDirectory, platform, defaultProfile).
141141
SetupWithManager(mgr)
142142
if err != nil {
143143
setupLog.Error(err, "unable to create controller", "controller", "Istio")
144144
os.Exit(1)
145145
}
146146

147-
err = remoteistio.NewReconciler(mgr.GetClient(), mgr.GetScheme(), resourceDirectory, defaultProfile).
147+
err = remoteistio.NewReconciler(mgr.GetClient(), mgr.GetScheme(), resourceDirectory, platform, defaultProfile).
148148
SetupWithManager(mgr)
149149
if err != nil {
150150
setupLog.Error(err, "unable to create controller", "controller", "RemoteIstio")
@@ -158,7 +158,7 @@ func main() {
158158
os.Exit(1)
159159
}
160160

161-
err = istiocni.NewReconciler(mgr.GetClient(), mgr.GetScheme(), resourceDirectory, chartManager, defaultProfile).
161+
err = istiocni.NewReconciler(mgr.GetClient(), mgr.GetScheme(), resourceDirectory, chartManager, platform, defaultProfile).
162162
SetupWithManager(mgr)
163163
if err != nil {
164164
setupLog.Error(err, "unable to create controller", "controller", "IstioCNI")

‎controllers/istio/istio_controller.go‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ import (
2424

2525
"github.com/go-logr/logr"
2626
"github.com/istio-ecosystem/sail-operator/api/v1alpha1"
27+
"github.com/istio-ecosystem/sail-operator/pkg/config"
2728
"github.com/istio-ecosystem/sail-operator/pkg/enqueuelogger"
2829
"github.com/istio-ecosystem/sail-operator/pkg/errlist"
2930
"github.com/istio-ecosystem/sail-operator/pkg/kube"
@@ -46,14 +47,16 @@ import (
4647
// Reconciler reconciles an Istio object
4748
type Reconciler struct {
4849
ResourceDirectory string
50+
Platform config.Platform
4951
DefaultProfile string
5052
client.Client
5153
Scheme *runtime.Scheme
5254
}
5355

54-
func NewReconciler(client client.Client, scheme *runtime.Scheme, resourceDir string, defaultProfile string) *Reconciler {
56+
func NewReconciler(client client.Client, scheme *runtime.Scheme, resourceDir string, platform config.Platform, defaultProfile string) *Reconciler {
5557
return &Reconciler{
5658
ResourceDirectory: resourceDir,
59+
Platform: platform,
5760
DefaultProfile: defaultProfile,
5861
Client: client,
5962
Scheme: scheme,
@@ -108,7 +111,7 @@ func validate(istio *v1alpha1.Istio) error {
108111
func (r *Reconciler) reconcileActiveRevision(ctx context.Context, istio *v1alpha1.Istio) error {
109112
values, err := revision.ComputeValues(
110113
istio.Spec.Values, istio.Spec.Namespace, istio.Spec.Version,
111-
r.DefaultProfile, istio.Spec.Profile,
114+
r.Platform, r.DefaultProfile, istio.Spec.Profile,
112115
r.ResourceDirectory, getActiveRevisionName(istio))
113116
if err != nil {
114117
return err

‎controllers/istio/istio_controller_test.go‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ import (
2323

2424
"github.com/google/go-cmp/cmp"
2525
"github.com/istio-ecosystem/sail-operator/api/v1alpha1"
26+
"github.com/istio-ecosystem/sail-operator/pkg/config"
2627
"github.com/istio-ecosystem/sail-operator/pkg/scheme"
2728
"github.com/istio-ecosystem/sail-operator/pkg/test/testtime"
2829
"github.com/istio-ecosystem/sail-operator/pkg/test/util/supportedversion"
@@ -60,7 +61,7 @@ func TestReconcile(t *testing.T) {
6061
cl := newFakeClientBuilder().
6162
WithObjects(istio).
6263
Build()
63-
reconciler := NewReconciler(cl, scheme.Scheme, resourceDir, "")
64+
reconciler := NewReconciler(cl, scheme.Scheme, resourceDir, config.PlatformKubernetes, "")
6465

6566
_, err := reconciler.Reconcile(ctx, istio)
6667
if err == nil {
@@ -96,7 +97,7 @@ func TestReconcile(t *testing.T) {
9697
WithStatusSubresource(&v1alpha1.Istio{}).
9798
WithObjects(istio).
9899
Build()
99-
reconciler := NewReconciler(cl, scheme.Scheme, resourceDir, "invalid-profile")
100+
reconciler := NewReconciler(cl, scheme.Scheme, resourceDir, config.PlatformKubernetes, "invalid-profile")
100101

101102
_, err := reconciler.Reconcile(ctx, istio)
102103
if err == nil {
@@ -136,7 +137,7 @@ func TestReconcile(t *testing.T) {
136137
},
137138
}).
138139
Build()
139-
reconciler := NewReconciler(cl, scheme.Scheme, resourceDir, "")
140+
reconciler := NewReconciler(cl, scheme.Scheme, resourceDir, config.PlatformKubernetes, "")
140141

141142
_, err := reconciler.Reconcile(ctx, istio)
142143
if err == nil {
@@ -532,7 +533,7 @@ func TestDetermineStatus(t *testing.T) {
532533
WithObjects(initObjs...).
533534
WithInterceptorFuncs(interceptorFuncs).
534535
Build()
535-
reconciler := NewReconciler(cl, scheme.Scheme, resourceDir, "")
536+
reconciler := NewReconciler(cl, scheme.Scheme, resourceDir, config.PlatformKubernetes, "")
536537

537538
status, err := reconciler.determineStatus(ctx, istio, tc.reconciliationErr)
538539
if (err != nil) != tc.wantErr {
@@ -733,7 +734,7 @@ func TestUpdateStatus(t *testing.T) {
733734
WithObjects(initObjs...).
734735
WithInterceptorFuncs(interceptorFuncs).
735736
Build()
736-
reconciler := NewReconciler(cl, scheme.Scheme, resourceDir, "")
737+
reconciler := NewReconciler(cl, scheme.Scheme, resourceDir, config.PlatformKubernetes, "")
737738

738739
err := reconciler.updateStatus(ctx, istio, tc.reconciliationErr)
739740
if (err != nil) != tc.wantErr {

‎controllers/istiocni/istiocni_controller.go‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -57,17 +57,19 @@ const (
5757
// Reconciler reconciles an IstioCNI object
5858
type Reconciler struct {
5959
ResourceDirectory string
60+
Platform config.Platform
6061
DefaultProfile string
6162
client.Client
6263
Scheme *runtime.Scheme
6364
ChartManager *helm.ChartManager
6465
}
6566

6667
func NewReconciler(
67-
client client.Client, scheme *runtime.Scheme, resourceDir string, chartManager *helm.ChartManager, defaultProfile string,
68+
client client.Client, scheme *runtime.Scheme, resourceDir string, chartManager *helm.ChartManager, platform config.Platform, defaultProfile string,
6869
) *Reconciler {
6970
return &Reconciler{
7071
ResourceDirectory: resourceDir,
72+
Platform: platform,
7173
DefaultProfile: defaultProfile,
7274
Client: client,
7375
Scheme: scheme,
@@ -150,7 +152,8 @@ func (r *Reconciler) installHelmChart(ctx context.Context, cni *v1alpha1.IstioCN
150152
userValues = applyImageDigests(cni, userValues, config.Config)
151153

152154
// apply userValues on top of defaultValues from profiles
153-
mergedHelmValues, err := istiovalues.ApplyProfiles(r.ResourceDirectory, cni.Spec.Version, r.DefaultProfile, cni.Spec.Profile, helm.FromValues(userValues))
155+
mergedHelmValues, err := istiovalues.ApplyProfilesAndPlatform(
156+
r.ResourceDirectory, cni.Spec.Version, r.Platform, r.DefaultProfile, cni.Spec.Profile, helm.FromValues(userValues))
154157
if err != nil {
155158
return fmt.Errorf("failed to apply profile: %w", err)
156159
}

‎controllers/istiocni/istiocni_controller_test.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,7 @@ func TestValidate(t *testing.T) {
107107
t.Run(tc.name, func(t *testing.T) {
108108
g := NewWithT(t)
109109
cl := fake.NewClientBuilder().WithScheme(scheme.Scheme).WithObjects(tc.objects...).Build()
110-
r := NewReconciler(cl, scheme.Scheme, "", nil, "")
110+
r := NewReconciler(cl, scheme.Scheme, "", nil, config.PlatformKubernetes, "")
111111

112112
err := r.validate(context.TODO(), tc.cni)
113113
if tc.expectErr == "" {
@@ -282,7 +282,7 @@ func TestDetermineReadyCondition(t *testing.T) {
282282

283283
cl := fake.NewClientBuilder().WithScheme(scheme.Scheme).WithObjects(tt.clientObjects...).WithInterceptorFuncs(tt.interceptors).Build()
284284

285-
r := NewReconciler(cl, scheme.Scheme, resourceDir, nil, "")
285+
r := NewReconciler(cl, scheme.Scheme, resourceDir, nil, config.PlatformKubernetes, "")
286286

287287
cni := &v1alpha1.IstioCNI{
288288
ObjectMeta: metav1.ObjectMeta{
@@ -464,7 +464,7 @@ func TestDetermineStatus(t *testing.T) {
464464
ctx := context.TODO()
465465
resourceDir := t.TempDir()
466466
cl := fake.NewClientBuilder().WithScheme(scheme.Scheme).Build()
467-
r := NewReconciler(cl, scheme.Scheme, resourceDir, nil, "")
467+
r := NewReconciler(cl, scheme.Scheme, resourceDir, nil, config.PlatformKubernetes, "")
468468

469469
for _, tt := range tests {
470470
t.Run(tt.name, func(t *testing.T) {

‎controllers/remoteistio/remoteistio_controller.go‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ import (
2424

2525
"github.com/go-logr/logr"
2626
"github.com/istio-ecosystem/sail-operator/api/v1alpha1"
27+
"github.com/istio-ecosystem/sail-operator/pkg/config"
2728
"github.com/istio-ecosystem/sail-operator/pkg/errlist"
2829
"github.com/istio-ecosystem/sail-operator/pkg/kube"
2930
"github.com/istio-ecosystem/sail-operator/pkg/reconciler"
@@ -44,14 +45,16 @@ import (
4445
// Reconciler reconciles a RemoteIstio object
4546
type Reconciler struct {
4647
ResourceDirectory string
48+
Platform config.Platform
4749
DefaultProfile string
4850
client.Client
4951
Scheme *runtime.Scheme
5052
}
5153

52-
func NewReconciler(client client.Client, scheme *runtime.Scheme, resourceDir string, defaultProfile string) *Reconciler {
54+
func NewReconciler(client client.Client, scheme *runtime.Scheme, resourceDir string, platform config.Platform, defaultProfile string) *Reconciler {
5355
return &Reconciler{
5456
ResourceDirectory: resourceDir,
57+
Platform: platform,
5558
DefaultProfile: defaultProfile,
5659
Client: client,
5760
Scheme: scheme,
@@ -106,7 +109,7 @@ func validate(istio *v1alpha1.RemoteIstio) error {
106109
func (r *Reconciler) reconcileActiveRevision(ctx context.Context, istio *v1alpha1.RemoteIstio) error {
107110
values, err := revision.ComputeValues(
108111
istio.Spec.Values, istio.Spec.Namespace, istio.Spec.Version,
109-
r.DefaultProfile, istio.Spec.Profile,
112+
r.Platform, r.DefaultProfile, istio.Spec.Profile,
110113
r.ResourceDirectory, getActiveRevisionName(istio))
111114
if err != nil {
112115
return err

‎controllers/remoteistio/remoteistio_controller_test.go‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ import (
2323

2424
"github.com/google/go-cmp/cmp"
2525
"github.com/istio-ecosystem/sail-operator/api/v1alpha1"
26+
"github.com/istio-ecosystem/sail-operator/pkg/config"
2627
"github.com/istio-ecosystem/sail-operator/pkg/scheme"
2728
"github.com/istio-ecosystem/sail-operator/pkg/test/testtime"
2829
"github.com/istio-ecosystem/sail-operator/pkg/test/util/supportedversion"
@@ -60,7 +61,7 @@ func TestReconcile(t *testing.T) {
6061
cl := newFakeClientBuilder().
6162
WithObjects(istio).
6263
Build()
63-
reconciler := NewReconciler(cl, scheme.Scheme, resourceDir, "")
64+
reconciler := NewReconciler(cl, scheme.Scheme, resourceDir, config.PlatformKubernetes, "")
6465

6566
_, err := reconciler.Reconcile(ctx, istio)
6667
if err == nil {
@@ -96,7 +97,7 @@ func TestReconcile(t *testing.T) {
9697
WithStatusSubresource(&v1alpha1.RemoteIstio{}).
9798
WithObjects(istio).
9899
Build()
99-
reconciler := NewReconciler(cl, scheme.Scheme, resourceDir, "invalid-profile")
100+
reconciler := NewReconciler(cl, scheme.Scheme, resourceDir, config.PlatformKubernetes, "invalid-profile")
100101

101102
_, err := reconciler.Reconcile(ctx, istio)
102103
if err == nil {
@@ -136,7 +137,7 @@ func TestReconcile(t *testing.T) {
136137
},
137138
}).
138139
Build()
139-
reconciler := NewReconciler(cl, scheme.Scheme, resourceDir, "")
140+
reconciler := NewReconciler(cl, scheme.Scheme, resourceDir, config.PlatformKubernetes, "")
140141

141142
_, err := reconciler.Reconcile(ctx, istio)
142143
if err == nil {
@@ -532,7 +533,7 @@ func TestDetermineStatus(t *testing.T) {
532533
WithObjects(initObjs...).
533534
WithInterceptorFuncs(interceptorFuncs).
534535
Build()
535-
reconciler := NewReconciler(cl, scheme.Scheme, resourceDir, "")
536+
reconciler := NewReconciler(cl, scheme.Scheme, resourceDir, config.PlatformKubernetes, "")
536537

537538
status, err := reconciler.determineStatus(ctx, istio, tc.reconciliationErr)
538539
if (err != nil) != tc.wantErr {
@@ -733,7 +734,7 @@ func TestUpdateStatus(t *testing.T) {
733734
WithObjects(initObjs...).
734735
WithInterceptorFuncs(interceptorFuncs).
735736
Build()
736-
reconciler := NewReconciler(cl, scheme.Scheme, resourceDir, "")
737+
reconciler := NewReconciler(cl, scheme.Scheme, resourceDir, config.PlatformKubernetes, "")
737738

738739
err := reconciler.updateStatus(ctx, istio, tc.reconciliationErr)
739740
if (err != nil) != tc.wantErr {

‎pkg/config/platform.go‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ import (
2525
type Platform string
2626

2727
const (
28+
PlatformUndefined Platform = ""
2829
PlatformOpenShift Platform = "openshift"
2930
PlatformKubernetes Platform = "kubernetes"
3031
)

‎pkg/helm/values.go‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,19 @@ func (h *Values) Set(key string, val any) error {
4242
return unstructured.SetNestedField(*h, val, toKeys(key)...)
4343
}
4444

45+
// SetIfAbsent sets the value of a nested field to a deep copy of the value
46+
// provided if the field does not exist.
47+
func (h *Values) SetIfAbsent(key string, val any) error {
48+
if _, found, err := unstructured.NestedFieldNoCopy(*h, toKeys(key)...); err != nil {
49+
return fmt.Errorf("failed to get value %s: %w", key, err)
50+
} else if !found {
51+
if err := h.Set(key, val); err != nil {
52+
return fmt.Errorf("failed to set value %s: %w", key, err)
53+
}
54+
}
55+
return nil
56+
}
57+
4558
func toKeys(key string) []string {
4659
return strings.Split(key, ".")
4760
}

‎pkg/helm/values_test.go‎

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -144,3 +144,71 @@ func TestSet(t *testing.T) {
144144
})
145145
}
146146
}
147+
148+
func TestSetIfAbsent(t *testing.T) {
149+
tests := []struct {
150+
name string
151+
input Values
152+
key string
153+
val string
154+
expected Values
155+
expectErr bool
156+
}{
157+
{
158+
name: "Key Exists",
159+
input: Values{
160+
"foo": map[string]any{
161+
"bar": "baz",
162+
},
163+
},
164+
key: "foo.bar",
165+
val: "newVal",
166+
expected: Values{
167+
"foo": map[string]any{
168+
"bar": "baz",
169+
},
170+
},
171+
},
172+
{
173+
name: "New Key",
174+
input: Values{
175+
"foo": map[string]any{
176+
"bar": "baz",
177+
},
178+
},
179+
key: "foo.baz",
180+
val: "newVal",
181+
expected: Values{
182+
"foo": map[string]any{
183+
"bar": "baz",
184+
"baz": "newVal",
185+
},
186+
},
187+
},
188+
{
189+
name: "Key Exists, but is not a map",
190+
input: Values{"foo": "bar"},
191+
key: "foo.baz",
192+
val: "newVal",
193+
expectErr: true,
194+
},
195+
}
196+
197+
for _, test := range tests {
198+
t.Run(test.name, func(t *testing.T) {
199+
err := test.input.SetIfAbsent(test.key, test.val)
200+
if test.expectErr {
201+
if err == nil {
202+
t.Errorf("Expected an error, but got nil")
203+
}
204+
} else {
205+
if err != nil {
206+
t.Errorf("Expected no error, but got an error: %v", err)
207+
}
208+
if !reflect.DeepEqual(test.input, test.expected) {
209+
t.Errorf("Expected %v, but got %v", test.expected, test.input)
210+
}
211+
}
212+
})
213+
}
214+
}

0 commit comments

Comments
 (0)