Skip to content

Commit

Permalink
apiserver observers: add observer mirrors writing to apiServerArguments
Browse files Browse the repository at this point in the history
  • Loading branch information
@stlaz
stlaz committed Mar 19, 2020
1 parent 256dedc commit fa68bbd
Show file tree
Hide file tree
Showing 4 changed files with 130 additions and 84 deletions.
15 changes: 12 additions & 3 deletions pkg/operator/configobserver/apiserver/observe_cors.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,18 @@ var clusterDefaultCORSAllowedOrigins = []string{
}

// ObserveAdditionalCORSAllowedOrigins observes the additionalCORSAllowedOrigins field
// of the APIServer resource
func ObserveAdditionalCORSAllowedOrigins(genericListers configobserver.Listers, recorder events.Recorder, existingConfig map[string]interface{}) (ret map[string]interface{}, _ []error) {
corsAllowedOriginsPath := []string{"corsAllowedOrigins"}
// of the APIServer resource and sets the corsAllowedOrigins field of observedConfig
func ObserveAdditionalCORSAllowedOrigins(genericListers configobserver.Listers, recorder events.Recorder, existingConfig map[string]interface{}) (map[string]interface{}, []error) {
return innerObserveAdditionalCORSAllowedOrigins(genericListers, recorder, existingConfig, []string{"corsAllowedOrigins"})
}

// ObserveAdditionalCORSAllowedOriginsToArguments observes the additionalCORSAllowedOrigins field
// of the APIServer resource and sets the cors-allowed-origins field in observedConfig.apiServerArguments
func ObserveAdditionalCORSAllowedOriginsToArguments(genericListers configobserver.Listers, recorder events.Recorder, existingConfig map[string]interface{}) (map[string]interface{}, []error) {
return innerObserveAdditionalCORSAllowedOrigins(genericListers, recorder, existingConfig, []string{"apiServerArguments", "cors-allowed-origins"})
}

func innerObserveAdditionalCORSAllowedOrigins(genericListers configobserver.Listers, recorder events.Recorder, existingConfig map[string]interface{}, corsAllowedOriginsPath []string) (ret map[string]interface{}, _ []error) {
defer func() {
ret = configobserver.Pruned(ret, corsAllowedOriginsPath)
}()
Expand Down
67 changes: 41 additions & 26 deletions pkg/operator/configobserver/apiserver/observe_cors_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
configlistersv1 "github.com/openshift/client-go/config/listers/config/v1"
"github.com/openshift/library-go/pkg/operator/events"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
"k8s.io/client-go/tools/cache"
)

Expand Down Expand Up @@ -42,36 +43,50 @@ func TestObserveAdditionalCORSAllowedOrigins(t *testing.T) {
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
indexer := cache.NewIndexer(cache.MetaNamespaceKeyFunc, cache.Indexers{})
if tt.corsConfig != nil {
if err := indexer.Add(&configv1.APIServer{
ObjectMeta: metav1.ObjectMeta{
Name: "cluster",
},
Spec: configv1.APIServerSpec{
AdditionalCORSAllowedOrigins: tt.corsConfig,
},
}); err != nil {
t.Fatal(err)
}
}
listers := testLister{
lister: configlistersv1.NewAPIServerLister(indexer),
for _, useAPIServerArguments := range []bool{false, true} {
var corsPath []string
if useAPIServerArguments {
corsPath = []string{"apiServerArguments", "cors-allowed-origins"}
} else {
corsPath = []string{"corsAllowedOrigins"}
}
t.Run(tt.name, func(t *testing.T) {
indexer := cache.NewIndexer(cache.MetaNamespaceKeyFunc, cache.Indexers{})
if tt.corsConfig != nil {
if err := indexer.Add(&configv1.APIServer{
ObjectMeta: metav1.ObjectMeta{
Name: "cluster",
},
Spec: configv1.APIServerSpec{
AdditionalCORSAllowedOrigins: tt.corsConfig,
},
}); err != nil {
t.Fatal(err)
}
}
listers := testLister{
lister: configlistersv1.NewAPIServerLister(indexer),
}

gotConfig, errs := ObserveAdditionalCORSAllowedOrigins(listers, events.NewInMemoryRecorder(t.Name()), tt.existingConfig)
if len(errs) > 0 {
t.Errorf("ObserveAdditionalCORSAllowedOrigins() expected no errors, got %v", errs)
}
var gotConfig map[string]interface{}
var errs []error
if useAPIServerArguments {
gotConfig, errs = ObserveAdditionalCORSAllowedOriginsToArguments(listers, events.NewInMemoryRecorder(t.Name()), tt.existingConfig)
} else {
gotConfig, errs = ObserveAdditionalCORSAllowedOrigins(listers, events.NewInMemoryRecorder(t.Name()), tt.existingConfig)
}
if len(errs) > 0 {
t.Errorf("expected no errors, got %v", errs)
}

gotCors := gotConfig["corsAllowedOrigins"].([]interface{})
for i := range tt.expectedCORS {
if gotCors[i] != tt.expectedCORS[i] {
t.Fatalf("ObserveAdditionalCORSAllowedOrigins() got = %v, want %v", gotCors, tt.expectedCORS)
gotCors, _, _ := unstructured.NestedStringSlice(gotConfig, corsPath...)
for i := range tt.expectedCORS {
if gotCors[i] != tt.expectedCORS[i] {
t.Fatalf("got = %v, want %v", gotCors, tt.expectedCORS)
}
}
}
})
})
}
}
}

Expand Down
28 changes: 16 additions & 12 deletions pkg/operator/configobserver/apiserver/observe_tlssecurityprofile.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,20 +25,26 @@ type APIServerLister interface {
}

// ObserveTLSSecurityProfile observes APIServer.Spec.TLSSecurityProfile field and sets
// the ServingInfo.MinTLSVersion, ServingInfo.CipherSuites fields
func ObserveTLSSecurityProfile(genericListers configobserver.Listers, recorder events.Recorder, existingConfig map[string]interface{}) (ret map[string]interface{}, _ []error) {
var (
minTlSVersionPath = []string{"servingInfo", "minTLSVersion"}
cipherSuitesPath = []string{"servingInfo", "cipherSuites"}
)
// the ServingInfo.MinTLSVersion, ServingInfo.CipherSuites fields of observed config
func ObserveTLSSecurityProfile(genericListers configobserver.Listers, recorder events.Recorder, existingConfig map[string]interface{}) (map[string]interface{}, []error) {
return innerTLSSecurityProfileObservations(genericListers, recorder, existingConfig, []string{"servingInfo", "minTLSVersion"}, []string{"servingInfo", "cipherSuites"})
}

// ObserveTLSSecurityProfileToArguments observes APIServer.Spec.TLSSecurityProfile field and sets
// the tls-min-version and tls-cipher-suites fileds of observedConfig.apiServerArguments
func ObserveTLSSecurityProfileToArguments(genericListers configobserver.Listers, recorder events.Recorder, existingConfig map[string]interface{}) (map[string]interface{}, []error) {
return innerTLSSecurityProfileObservations(genericListers, recorder, existingConfig, []string{"apiServerArguments", "tls-min-version"}, []string{"apiServerArguments", "tls-cipher-suites"})
}

func innerTLSSecurityProfileObservations(genericListers configobserver.Listers, recorder events.Recorder, existingConfig map[string]interface{}, minTLSVersionPath, cipherSuitesPath []string) (ret map[string]interface{}, _ []error) {
defer func() {
ret = configobserver.Pruned(ret, minTlSVersionPath, cipherSuitesPath)
ret = configobserver.Pruned(ret, minTLSVersionPath, cipherSuitesPath)
}()

listers := genericListers.(APIServerLister)
errs := []error{}

currentMinTLSVersion, _, versionErr := unstructured.NestedString(existingConfig, minTlSVersionPath...)
currentMinTLSVersion, _, versionErr := unstructured.NestedString(existingConfig, minTLSVersionPath...)
if versionErr != nil {
errs = append(errs, fmt.Errorf("failed to retrieve spec.servingInfo.minTLSVersion: %v", versionErr))
// keep going on read error from existing config
Expand All @@ -58,11 +64,9 @@ func ObserveTLSSecurityProfile(genericListers configobserver.Listers, recorder e
return existingConfig, append(errs, err)
}

observedConfig := map[string]interface{}{
"servingInfo": map[string]interface{}{},
}
observedConfig := map[string]interface{}{}
observedMinTLSVersion, observedCipherSuites := getSecurityProfileCiphers(apiServer.Spec.TLSSecurityProfile)
if err = unstructured.SetNestedField(observedConfig, observedMinTLSVersion, minTlSVersionPath...); err != nil {
if err = unstructured.SetNestedField(observedConfig, observedMinTLSVersion, minTLSVersionPath...); err != nil {
return existingConfig, append(errs, err)
}
if err = unstructured.SetNestedStringSlice(observedConfig, observedCipherSuites, cipherSuitesPath...); err != nil {
Expand Down
104 changes: 61 additions & 43 deletions pkg/operator/configobserver/apiserver/observe_tlssecurityprofile_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,22 +32,18 @@ func (l testLister) PreRunHasSynced() []cache.InformerSynced {
return nil
}
func TestObserveTLSSecurityProfile(t *testing.T) {
existingConfig := map[string]interface{}{
"minTLSVersion": "VersionTLS11",
"cipherSuites": []string{"DES-CBC3-SHA"},
}
existingTLSVersion := "VersionTLS11"
existingCipherSuites := []interface{}{"DES-CBC3-SHA"}

tests := []struct {
name string
config *configv1.TLSSecurityProfile
existing map[string]interface{}
expectedMinTLSVersion string
expectedSuites []string
}{
{
name: "NoAPIServerConfig",
config: nil,
existing: existingConfig,
expectedMinTLSVersion: "VersionTLS12",
expectedSuites: crypto.OpenSSLToIANACipherSuites(configv1.TLSProfiles[configv1.TLSProfileIntermediateType].Ciphers),
},
Expand All @@ -57,7 +53,6 @@ func TestObserveTLSSecurityProfile(t *testing.T) {
Type: configv1.TLSProfileModernType,
Modern: &configv1.ModernTLSProfile{},
},
existing: existingConfig,
expectedMinTLSVersion: "VersionTLS13",
expectedSuites: crypto.OpenSSLToIANACipherSuites(configv1.TLSProfiles[configv1.TLSProfileModernType].Ciphers),
},
Expand All @@ -67,51 +62,74 @@ func TestObserveTLSSecurityProfile(t *testing.T) {
Type: configv1.TLSProfileOldType,
Old: &configv1.OldTLSProfile{},
},
existing: existingConfig,
expectedMinTLSVersion: "VersionTLS10",
expectedSuites: crypto.OpenSSLToIANACipherSuites(configv1.TLSProfiles[configv1.TLSProfileOldType].Ciphers),
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
indexer := cache.NewIndexer(cache.MetaNamespaceKeyFunc, cache.Indexers{})
if tt.config != nil {
if err := indexer.Add(&configv1.APIServer{
ObjectMeta: metav1.ObjectMeta{
Name: "cluster",
},
Spec: configv1.APIServerSpec{
TLSSecurityProfile: tt.config,
},
}); err != nil {
t.Fatal(err)
}
}
listers := testLister{
lister: configlistersv1.NewAPIServerLister(indexer),
for _, useAPIServerArgs := range []bool{false, true} {
var minTLSVersionPath, cipherSuitesPath []string
if useAPIServerArgs {
minTLSVersionPath = []string{"apiServerArguments", "tls-min-version"}
cipherSuitesPath = []string{"apiServerArguments", "tls-cipher-suites"}
} else {
minTLSVersionPath = []string{"servingInfo", "minTLSVersion"}
cipherSuitesPath = []string{"servingInfo", "cipherSuites"}
}
t.Run(tt.name, func(t *testing.T) {
indexer := cache.NewIndexer(cache.MetaNamespaceKeyFunc, cache.Indexers{})
if tt.config != nil {
if err := indexer.Add(&configv1.APIServer{
ObjectMeta: metav1.ObjectMeta{
Name: "cluster",
},
Spec: configv1.APIServerSpec{
TLSSecurityProfile: tt.config,
},
}); err != nil {
t.Fatal(err)
}
}
listers := testLister{
lister: configlistersv1.NewAPIServerLister(indexer),
}

result, errs := ObserveTLSSecurityProfile(listers, events.NewInMemoryRecorder(t.Name()), tt.existing)
if len(errs) > 0 {
t.Errorf("expected 0 errors, got %v", errs)
}
existingConfig := map[string]interface{}{}
if err := unstructured.SetNestedField(existingConfig, existingTLSVersion, minTLSVersionPath...); err != nil {
t.Fatalf("couldn't set existing min TLS version: %v", err)
}
if err := unstructured.SetNestedField(existingConfig, existingCipherSuites, cipherSuitesPath...); err != nil {
t.Fatalf("couldn't set existing cipher suites: %v", err)
}

gotMinTLSVersion, _, err := unstructured.NestedString(result, "servingInfo", "minTLSVersion")
if err != nil {
t.Errorf("couldn't get minTLSVersion from the returned object: %v", err)
}
var result map[string]interface{}
var errs []error
if useAPIServerArgs {
result, errs = ObserveTLSSecurityProfileToArguments(listers, events.NewInMemoryRecorder(t.Name()), existingConfig)
} else {
result, errs = ObserveTLSSecurityProfile(listers, events.NewInMemoryRecorder(t.Name()), existingConfig)
}
if len(errs) > 0 {
t.Errorf("expected 0 errors, got %v", errs)
}

gotSuites, _, err := unstructured.NestedStringSlice(result, "servingInfo", "cipherSuites")
if err != nil {
t.Errorf("couldn't get cipherSuites from the returned object: %v", err)
}
gotMinTLSVersion, _, err := unstructured.NestedString(result, minTLSVersionPath...)
if err != nil {
t.Errorf("couldn't get minTLSVersion from the returned object: %v", err)
}

if !reflect.DeepEqual(gotSuites, tt.expectedSuites) {
t.Errorf("ObserveTLSSecurityProfile() got cipherSuites = %v, expected %v", gotSuites, tt.expectedSuites)
}
if gotMinTLSVersion != tt.expectedMinTLSVersion {
t.Errorf("ObserveTLSSecurityProfile() got minTlSVersion = %v, expected %v", gotMinTLSVersion, tt.expectedMinTLSVersion)
}
})
gotSuites, _, err := unstructured.NestedStringSlice(result, cipherSuitesPath...)
if err != nil {
t.Errorf("couldn't get cipherSuites from the returned object: %v", err)
}

if !reflect.DeepEqual(gotSuites, tt.expectedSuites) {
t.Errorf("got cipherSuites = %v, expected %v", gotSuites, tt.expectedSuites)
}
if gotMinTLSVersion != tt.expectedMinTLSVersion {
t.Errorf("got minTlSVersion = %v, expected %v", gotMinTLSVersion, tt.expectedMinTLSVersion)
}
})
}
}
}

0 comments on commit fa68bbd

Please sign in to comment.