Skip to content

Commit

Permalink
Merge pull request openshift#1631 from hasueki/fix-global-config-depr…
Browse files Browse the repository at this point in the history 
…ecation

fix(ho): honor deprecated global config fields
  • Loading branch information
@openshift-merge-robot
openshift-merge-robot authored Aug 1, 2022
2 parents eef8269 + c320791 commit 1792285
Show file tree
Hide file tree
Showing 3 changed files with 73 additions and 57 deletions.
56 changes: 11 additions & 45 deletions hypershift-operator/controllers/hostedcluster/hostedcluster_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3491,37 +3491,9 @@ func (r *HostedClusterReconciler) validateConfigAndClusterCapabilities(ctx conte

// TODO: Drop when we no longer need to support versions < 4.11
if hc.Spec.Configuration != nil {
globalConfig, err := globalconfig.ParseGlobalConfig(ctx, hc.Spec.Configuration)
_, err := globalconfig.ParseGlobalConfig(ctx, hc.Spec.Configuration)
if err != nil {
errs = append(errs, fmt.Errorf("failed to parse cluster configuration: %w", err))
} else {
if globalConfig.APIServer != nil && hc.Spec.Configuration.APIServer != nil {
errs = append(errs, fmt.Errorf("apiServer raw extension config is invalid when APIServer configuration field is set"))
}
if globalConfig.Authentication != nil && hc.Spec.Configuration.Authentication != nil {
errs = append(errs, fmt.Errorf("authentication raw extension config is invalid when Authentication configuration field is set"))
}
if globalConfig.FeatureGate != nil && hc.Spec.Configuration.FeatureGate != nil {
errs = append(errs, fmt.Errorf("featureGate raw extension config is invalid when FeatureGate configuration field is set"))
}
if globalConfig.Image != nil && hc.Spec.Configuration.Image != nil {
errs = append(errs, fmt.Errorf("image raw extension config is invalid when Image configuration field is set"))
}
if globalConfig.Ingress != nil && hc.Spec.Configuration.Ingress != nil {
errs = append(errs, fmt.Errorf("ingress raw extension config is invalid when Ingress configuration field is set"))
}
if globalConfig.Network != nil && hc.Spec.Configuration.Network != nil {
errs = append(errs, fmt.Errorf("network raw extension config is invalid when Network configuration field is set"))
}
if globalConfig.OAuth != nil && hc.Spec.Configuration.OAuth != nil {
errs = append(errs, fmt.Errorf("oAuth raw extension config is invalid when OAuth configuration field is set"))
}
if globalConfig.Proxy != nil && hc.Spec.Configuration.Proxy != nil {
errs = append(errs, fmt.Errorf("proxy raw extension config is invalid when Proxy configuration field is set"))
}
if globalConfig.Scheduler != nil && hc.Spec.Configuration.Scheduler != nil {
errs = append(errs, fmt.Errorf("scheduler raw extension config is invalid when Scheduler configuration field is set"))
}
}
}

Expand Down Expand Up @@ -4529,40 +4501,34 @@ func (r *HostedClusterReconciler) reconcileDeprecatedGlobalConfig(ctx context.Co
return err
}

// Only copy over config from the raw extension if the field is not already populated.
// Once populated, the field takes precedence and a conflicting raw extension config
// results in an invalid configuration field.

if gconfig.APIServer != nil && hc.Spec.Configuration.APIServer == nil {
// Copy over config from the raw extension
if gconfig.APIServer != nil {
hc.Spec.Configuration.APIServer = &gconfig.APIServer.Spec
}
if gconfig.Authentication != nil && hc.Spec.Configuration.Authentication == nil {
if gconfig.Authentication != nil {
hc.Spec.Configuration.Authentication = &gconfig.Authentication.Spec
}
if gconfig.FeatureGate != nil && hc.Spec.Configuration.FeatureGate == nil {
if gconfig.FeatureGate != nil {
hc.Spec.Configuration.FeatureGate = &gconfig.FeatureGate.Spec
}
if gconfig.Image != nil && hc.Spec.Configuration.Image == nil {
if gconfig.Image != nil {
hc.Spec.Configuration.Image = &gconfig.Image.Spec
}
if gconfig.Ingress != nil && hc.Spec.Configuration.Ingress == nil {
if gconfig.Ingress != nil {
hc.Spec.Configuration.Ingress = &gconfig.Ingress.Spec
}
if gconfig.Network != nil && hc.Spec.Configuration.Network == nil {
if gconfig.Network != nil {
hc.Spec.Configuration.Network = &gconfig.Network.Spec
}
if gconfig.OAuth != nil && hc.Spec.Configuration.OAuth == nil {
if gconfig.OAuth != nil {
hc.Spec.Configuration.OAuth = &gconfig.OAuth.Spec
}
if gconfig.Scheduler != nil && hc.Spec.Configuration.Scheduler == nil {
if gconfig.Scheduler != nil {
hc.Spec.Configuration.Scheduler = &gconfig.Scheduler.Spec
}
if gconfig.Proxy != nil && hc.Spec.Configuration.Proxy == nil {
if gconfig.Proxy != nil {
hc.Spec.Configuration.Proxy = &gconfig.Proxy.Spec
}
hc.Spec.Configuration.Items = nil
hc.Spec.Configuration.ConfigMapRefs = nil
hc.Spec.Configuration.SecretRefs = nil

return nil
}
Expand Down
69 changes: 57 additions & 12 deletions hypershift-operator/controllers/hostedcluster/hostedcluster_controller_test.go
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package hostedcluster

import (
"bytes"
"context"
"encoding/json"
"errors"
Expand All @@ -26,7 +27,6 @@ import (
fakereleaseprovider "github.com/openshift/hypershift/support/releaseinfo/fake"
"github.com/openshift/hypershift/support/thirdparty/library-go/pkg/image/dockerv1client"
"github.com/openshift/hypershift/support/upsert"
"github.com/openshift/hypershift/support/util"
"github.com/openshift/hypershift/support/util/fakeimagemetadataprovider"
"go.uber.org/zap/zapcore"
corev1 "k8s.io/api/core/v1"
Expand All @@ -36,6 +36,7 @@ import (
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
"k8s.io/apimachinery/pkg/runtime"
serializerjson "k8s.io/apimachinery/pkg/runtime/serializer/json"
"k8s.io/apimachinery/pkg/types"
"k8s.io/apimachinery/pkg/util/intstr"
"k8s.io/apimachinery/pkg/util/sets"
Expand Down Expand Up @@ -2149,30 +2150,52 @@ func TestReconcileDeprecatedGlobalConfig(t *testing.T) {
hc.Namespace = "fake-namespace"

apiServer := &configv1.APIServer{
TypeMeta: metav1.TypeMeta{
APIVersion: configv1.SchemeGroupVersion.String(),
Kind: "APIServer",
},
ObjectMeta: metav1.ObjectMeta{
Name: "cluster",
},
Spec: configv1.APIServerSpec{
Audit: configv1.Audit{
// Populate kubebuilder default for comparison
// https://github.com/openshift/api/blob/f120778bee805ad1a7a4f05a6430332cf5811813/config/v1/types_apiserver.go#L57
Profile: configv1.DefaultAuditProfileType,
},
ClientCA: configv1.ConfigMapNameReference{
Name: "fake-ca",
},
},
}
serializedAPIServer, err := util.SerializeResource(apiServer, hyperapi.Scheme)

jsonSerializer := serializerjson.NewSerializerWithOptions(
serializerjson.DefaultMetaFactory, hyperapi.Scheme, hyperapi.Scheme,
serializerjson.SerializerOptions{Yaml: false, Pretty: true, Strict: false},
)

serializedAPIServer := &bytes.Buffer{}
err := jsonSerializer.Encode(apiServer, serializedAPIServer)
if err != nil {
t.Fatalf("failed to serialize apiserver: %v", err)
}

hc.Spec.Configuration = &hyperv1.ClusterConfiguration{
Items: []runtime.RawExtension{
{
Raw: []byte(serializedAPIServer),
Raw: serializedAPIServer.Bytes(),
},
},
ConfigMapRefs: []corev1.LocalObjectReference{
{
Name: "fake-ca",
},
},
SecretRefs: []corev1.LocalObjectReference{
{
Name: "fake-creds",
},
},
}

fakeClient := fake.NewClientBuilder().
Expand All @@ -2192,30 +2215,52 @@ func TestReconcileDeprecatedGlobalConfig(t *testing.T) {
if !equality.Semantic.DeepEqual(&hc.Spec, originalSpec) {
err := reconciler.Client.Update(context.Background(), hc)
if err != nil {
t.Fatalf("unexpected error: %v", err)
t.Fatalf("unexpected update error: %v", err)
}
}

updatedHc := &hyperv1.HostedCluster{}
if err := fakeClient.Get(context.Background(), crclient.ObjectKeyFromObject(hc), updatedHc); err != nil {
t.Fatalf("unexpected error: %v", err)
t.Fatalf("unexpected get error: %v", err)
}
if updatedHc.Spec.Configuration == nil {
t.Fatalf("unexpected nil configuration")
}

if len(updatedHc.Spec.Configuration.Items) > 0 {
t.Errorf("non-empty deprecated configuration")
if len(updatedHc.Spec.Configuration.Items) == 0 {
t.Errorf("empty deprecated configuration")
}
if len(updatedHc.Spec.Configuration.ConfigMapRefs) > 0 {
t.Errorf("non-empty configmap refs")
if len(updatedHc.Spec.Configuration.ConfigMapRefs) == 0 {
t.Errorf("empty configmap refs")
}
if len(updatedHc.Spec.Configuration.SecretRefs) > 0 {
t.Errorf("non-emtpy secret refs")
if len(updatedHc.Spec.Configuration.SecretRefs) == 0 {
t.Errorf("emtpy secret refs")
}
if !equality.Semantic.DeepEqual(&apiServer.Spec, updatedHc.Spec.Configuration.APIServer) {
t.Errorf("unexpected apiserver spec: %#v", updatedHc.Spec.Configuration.APIServer)
}

// Update deprecated field, remove test when field is unsupported
apiServer.Spec.ClientCA.Name = "updated-ca"
serializedAPIServer.Reset()
err = jsonSerializer.Encode(apiServer, serializedAPIServer)
if err != nil {
t.Fatalf("failed to serialize apiserver: %v", err)
}
updatedHc.Spec.Configuration.Items = []runtime.RawExtension{{Raw: serializedAPIServer.Bytes()}}
if err := reconciler.reconcileDeprecatedGlobalConfig(context.Background(), updatedHc); err != nil {
t.Fatalf("unexpected reconcile error: %v", err)
}
err = reconciler.Client.Update(context.Background(), updatedHc)
if err != nil {
t.Fatalf("unexpected update error: %v", err)
}
updatedHcAgain := &hyperv1.HostedCluster{}
if err := fakeClient.Get(context.Background(), crclient.ObjectKeyFromObject(updatedHc), updatedHcAgain); err != nil {
t.Fatalf("unexpected get error: %v", err)
}
if !equality.Semantic.DeepEqual(&apiServer.Spec, updatedHcAgain.Spec.Configuration.APIServer) {
t.Errorf("unexpected apiserver spec on update: %#v", updatedHcAgain.Spec.Configuration.APIServer)
}
}

func TestReconciliationSuccessConditionSetting(t *testing.T) {
Expand Down
5 changes: 5 additions & 0 deletions support/globalconfig/globalconfig.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,11 @@ func ParseGlobalConfig(ctx context.Context, cfg *hyperv1.ClusterConfiguration) (
kinds.Insert(gvk.Kind)
switch obj := cfgObject.(type) {
case *configv1.APIServer:
if obj.Spec.Audit.Profile == "" {
// Populate kubebuilder default for comparison
// https://github.com/openshift/api/blob/f120778bee805ad1a7a4f05a6430332cf5811813/config/v1/types_apiserver.go#L57
obj.Spec.Audit.Profile = configv1.DefaultAuditProfileType
}
globalConfig.APIServer = obj
case *configv1.Authentication:
globalConfig.Authentication = obj
Expand Down

0 comments on commit 1792285

Please sign in to comment.