diff --git a/apis/v1/jaeger_types.go b/apis/v1/jaeger_types.go index c5b21df71..cac5ba547 100644 --- a/apis/v1/jaeger_types.go +++ b/apis/v1/jaeger_types.go @@ -54,6 +54,9 @@ const ( // FlagKafkaProvision represents the 'kafka-provision' flag. FlagKafkaProvision = "kafka-provision" + // FlagAuthDelegatorAvailability represents the 'auth-delegator-available' flag. + FlagAuthDelegatorAvailability = "auth-delegator-available" + // IngressSecurityNone disables any form of security for ingress objects (default) IngressSecurityNone IngressSecurityType = "" diff --git a/pkg/autodetect/main.go b/pkg/autodetect/main.go index d0f236135..e95466153 100644 --- a/pkg/autodetect/main.go +++ b/pkg/autodetect/main.go @@ -356,32 +356,32 @@ func (b *Background) detectClusterRoles(ctx context.Context) { Token: "TEST", }, } - currentAuthDelegator := viper.GetBool("auth-delegator-available") - var newAuthDelegator bool + currentAuthDelegator := OperatorConfiguration.GetAuthDelegator() + var newAuthDelegator AuthDelegatorAvailability if err := b.cl.Create(ctx, tr); err != nil { - if !viper.IsSet("auth-delegator-available") || (viper.IsSet("auth-delegator-available") && currentAuthDelegator) { + if !OperatorConfiguration.IsAuthDelegatorSet() || OperatorConfiguration.IsAuthDelegatorAvailable() { // for the first run, we log this info, or when the previous value was true log.Log.Info( "The service account running this operator does not have the role 'system:auth-delegator', consider granting it for additional capabilities", ) } - newAuthDelegator = false + newAuthDelegator = AuthDelegatorAvailabilityNo } else { // this isn't technically correct, as we only ensured that we can create token reviews (which is what the OAuth Proxy does) // but it might be the case that we have *another* cluster role that includes this access and still not have // the "system:auth-delegator". This is an edge case, and it's more complicated to check that, so, we'll keep it simple for now // and deal with the edge case if it ever manifests in the real world - if !viper.IsSet("auth-delegator-available") || (viper.IsSet("auth-delegator-available") && !currentAuthDelegator) { + if !OperatorConfiguration.IsAuthDelegatorSet() || (OperatorConfiguration.IsAuthDelegatorSet() && !OperatorConfiguration.IsAuthDelegatorAvailable()) { // for the first run, we log this info, or when the previous value was 'false' log.Log.Info( "The service account running this operator has the role 'system:auth-delegator', enabling OAuth Proxy's 'delegate-urls' option", ) } - newAuthDelegator = true + newAuthDelegator = AuthDelegatorAvailabilityYes } - if currentAuthDelegator != newAuthDelegator || !viper.IsSet("auth-delegator-available") { - viper.Set("auth-delegator-available", newAuthDelegator) + if currentAuthDelegator != newAuthDelegator || !OperatorConfiguration.IsAuthDelegatorSet() { + OperatorConfiguration.SetAuthDelegatorAvailability(newAuthDelegator) } if err := b.cl.Delete(ctx, tr); err != nil { diff --git a/pkg/autodetect/main_test.go b/pkg/autodetect/main_test.go index 0cb0d914a..6bb2d9b48 100644 --- a/pkg/autodetect/main_test.go +++ b/pkg/autodetect/main_test.go @@ -25,7 +25,7 @@ func TestStart(t *testing.T) { defer viper.Reset() // sanity check - assert.False(t, viper.IsSet("auth-delegator-available")) + assert.False(t, OperatorConfiguration.IsAuthDelegatorAvailable()) // prepare dcl := &fakeDiscoveryClient{} @@ -35,7 +35,7 @@ func TestStart(t *testing.T) { done := make(chan bool) go func() { for { - if viper.IsSet("auth-delegator-available") { + if OperatorConfiguration.IsAuthDelegatorSet() { break } // it would typically take less than 10ms to get the first result already, so, it should wait only once @@ -50,7 +50,7 @@ func TestStart(t *testing.T) { // verify select { case <-done: - assert.True(t, viper.GetBool("auth-delegator-available")) + assert.True(t, OperatorConfiguration.IsAuthDelegatorAvailable()) case <-time.After(1 * time.Second): assert.Fail(t, "timed out waiting for the start process to detect the capabilities") } @@ -71,7 +71,7 @@ func TestStartContinuesInBackground(t *testing.T) { done := make(chan bool) go func() { for { - if viper.IsSet("auth-delegator-available") { + if OperatorConfiguration.IsAuthDelegatorSet() { break } // it would typically take less than 10ms to get the first result already, so, it should wait only once @@ -84,7 +84,7 @@ func TestStartContinuesInBackground(t *testing.T) { select { case <-done: - assert.False(t, viper.GetBool("auth-delegator-available")) + assert.False(t, OperatorConfiguration.IsAuthDelegatorAvailable()) case <-time.After(1 * time.Second): assert.Fail(t, "timed out waiting for the start process to detect the capabilities") } @@ -94,7 +94,7 @@ func TestStartContinuesInBackground(t *testing.T) { go func() { for { - if viper.GetBool("auth-delegator-available") { + if OperatorConfiguration.IsAuthDelegatorAvailable() { break } time.Sleep(500 * time.Millisecond) @@ -105,7 +105,7 @@ func TestStartContinuesInBackground(t *testing.T) { // verify select { case <-done: - assert.True(t, viper.GetBool("auth-delegator-available")) + assert.True(t, OperatorConfiguration.IsAuthDelegatorAvailable()) case <-time.After(6 * time.Second): // this one might take up to 5 seconds to run again + processing time assert.Fail(t, "timed out waiting for the start process to detect the new capabilities") } @@ -510,7 +510,7 @@ func TestSkipAuthDelegatorNonOpenShift(t *testing.T) { b.detectClusterRoles(context.Background()) // verify - assert.False(t, viper.IsSet("auth-delegator-available")) + assert.False(t, OperatorConfiguration.IsAuthDelegatorAvailable()) } func TestNoAuthDelegatorAvailable(t *testing.T) { @@ -529,7 +529,7 @@ func TestNoAuthDelegatorAvailable(t *testing.T) { b.detectClusterRoles(context.Background()) // verify - assert.False(t, viper.GetBool("auth-delegator-available")) + assert.False(t, OperatorConfiguration.IsAuthDelegatorAvailable()) } func TestAuthDelegatorBecomesAvailable(t *testing.T) { @@ -546,11 +546,11 @@ func TestAuthDelegatorBecomesAvailable(t *testing.T) { // test b.detectClusterRoles(context.Background()) - assert.False(t, viper.GetBool("auth-delegator-available")) + assert.False(t, OperatorConfiguration.IsAuthDelegatorAvailable()) cl.CreateFunc = cl.Client.Create b.detectClusterRoles(context.Background()) - assert.True(t, viper.GetBool("auth-delegator-available")) + assert.True(t, OperatorConfiguration.IsAuthDelegatorAvailable()) } func TestAuthDelegatorBecomesUnavailable(t *testing.T) { @@ -564,13 +564,13 @@ func TestAuthDelegatorBecomesUnavailable(t *testing.T) { // test b.detectClusterRoles(context.Background()) - assert.True(t, viper.GetBool("auth-delegator-available")) + assert.True(t, OperatorConfiguration.IsAuthDelegatorAvailable()) cl.CreateFunc = func(ctx context.Context, obj client.Object, opts ...client.CreateOption) error { return fmt.Errorf("faked error") } b.detectClusterRoles(context.Background()) - assert.False(t, viper.GetBool("auth-delegator-available")) + assert.False(t, OperatorConfiguration.IsAuthDelegatorAvailable()) } type fakeClient struct { diff --git a/pkg/autodetect/operatorconfig.go b/pkg/autodetect/operatorconfig.go index 7b52f22f7..5ea770c5d 100644 --- a/pkg/autodetect/operatorconfig.go +++ b/pkg/autodetect/operatorconfig.go @@ -54,6 +54,24 @@ func (p KafkaOperatorIntegration) String() string { return [...]string{"Yes", "No"}[p] } +// AuthDelegatorAvailability holds the if the AuthDelegator available. +type AuthDelegatorAvailability int + +const ( + // AuthDelegatorAvailabilityYes represents the AuthDelegator is available. + AuthDelegatorAvailabilityYes AuthDelegatorAvailability = iota + + // AuthDelegatorAvailabilityNo represents the AuthDelegator is not available. + AuthDelegatorAvailabilityNo + + // AuthDelegatorAvailabilityUnknown represents the AuthDelegator availability is not known. + AuthDelegatorAvailabilityUnknown +) + +func (p AuthDelegatorAvailability) String() string { + return [...]string{"Yes", "No", "Unknown"}[p] +} + var OperatorConfiguration operatorConfigurationWrapper type operatorConfigurationWrapper struct { @@ -160,3 +178,46 @@ func (c *operatorConfigurationWrapper) GetKafkaIntegration() KafkaOperatorIntegr func (c *operatorConfigurationWrapper) IsKafkaOperatorIntegrationEnabled() bool { return c.GetKafkaIntegration() == KafkaOperatorIntegrationYes } + +func (c *operatorConfigurationWrapper) SetAuthDelegatorAvailability(e interface{}) { + var availability string + switch v := e.(type) { + case string: + availability = v + case AuthDelegatorAvailability: + availability = v.String() + default: + availability = AuthDelegatorAvailabilityUnknown.String() + } + + c.mu.Lock() + viper.Set(v1.FlagAuthDelegatorAvailability, availability) + c.mu.Unlock() +} + +func (c *operatorConfigurationWrapper) GetAuthDelegator() AuthDelegatorAvailability { + c.mu.RLock() + e := viper.GetString(v1.FlagAuthDelegatorAvailability) + c.mu.RUnlock() + + var available AuthDelegatorAvailability + switch strings.ToLower(e) { + case "yes": + available = AuthDelegatorAvailabilityYes + case "no": + available = AuthDelegatorAvailabilityNo + default: + available = AuthDelegatorAvailabilityUnknown + } + return available +} + +// IsAuthDelegatorAvailable returns true if the AuthDelegator is available +func (c *operatorConfigurationWrapper) IsAuthDelegatorAvailable() bool { + return c.GetAuthDelegator() == AuthDelegatorAvailabilityYes +} + +// IsAuthDelegatorAvailable returns true if the AuthDelegator is set +func (c *operatorConfigurationWrapper) IsAuthDelegatorSet() bool { + return c.GetAuthDelegator() != AuthDelegatorAvailabilityUnknown +} diff --git a/pkg/clusterrolebinding/main.go b/pkg/clusterrolebinding/main.go index ea1a68e75..94ccaa4bc 100644 --- a/pkg/clusterrolebinding/main.go +++ b/pkg/clusterrolebinding/main.go @@ -3,19 +3,19 @@ package clusterrolebinding import ( "fmt" - "github.com/spf13/viper" rbac "k8s.io/api/rbac/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" v1 "github.com/jaegertracing/jaeger-operator/apis/v1" "github.com/jaegertracing/jaeger-operator/pkg/account" + "github.com/jaegertracing/jaeger-operator/pkg/autodetect" "github.com/jaegertracing/jaeger-operator/pkg/util" ) // Get returns all the service accounts to be created for this Jaeger instance func Get(jaeger *v1.Jaeger) []rbac.ClusterRoleBinding { if jaeger.Spec.Ingress.Security == v1.IngressSecurityOAuthProxy && len(jaeger.Spec.Ingress.Openshift.DelegateUrls) > 0 { - if viper.GetBool("auth-delegator-available") { + if autodetect.OperatorConfiguration.IsAuthDelegatorAvailable() { return []rbac.ClusterRoleBinding{oauthProxyAuthDelegator(jaeger)} } diff --git a/pkg/clusterrolebinding/main_test.go b/pkg/clusterrolebinding/main_test.go index 6f46e8143..4d79598bd 100644 --- a/pkg/clusterrolebinding/main_test.go +++ b/pkg/clusterrolebinding/main_test.go @@ -9,6 +9,7 @@ import ( v1 "github.com/jaegertracing/jaeger-operator/apis/v1" "github.com/jaegertracing/jaeger-operator/pkg/account" + "github.com/jaegertracing/jaeger-operator/pkg/autodetect" ) func TestGetClusterRoleBinding(t *testing.T) { @@ -16,7 +17,7 @@ func TestGetClusterRoleBinding(t *testing.T) { name := "TestGetClusterRoleBinding" trueVar := true - viper.Set("auth-delegator-available", true) + autodetect.OperatorConfiguration.SetAuthDelegatorAvailability(autodetect.AuthDelegatorAvailabilityYes) defer viper.Reset() jaeger := v1.NewJaeger(types.NamespacedName{Name: name}) @@ -77,7 +78,7 @@ func TestAuthDelegatorNotAvailable(t *testing.T) { name := "TestAuthDelegatorNotAvailable" trueVar := true - viper.Set("auth-delegator-available", false) + autodetect.OperatorConfiguration.SetAuthDelegatorAvailability(autodetect.AuthDelegatorAvailabilityNo) defer viper.Reset() jaeger := v1.NewJaeger(types.NamespacedName{Name: name}) diff --git a/pkg/inject/oauth_proxy.go b/pkg/inject/oauth_proxy.go index 7032adea9..4c4a7277f 100644 --- a/pkg/inject/oauth_proxy.go +++ b/pkg/inject/oauth_proxy.go @@ -11,6 +11,7 @@ import ( v1 "github.com/jaegertracing/jaeger-operator/apis/v1" "github.com/jaegertracing/jaeger-operator/pkg/account" + "github.com/jaegertracing/jaeger-operator/pkg/autodetect" "github.com/jaegertracing/jaeger-operator/pkg/config/ca" "github.com/jaegertracing/jaeger-operator/pkg/service" "github.com/jaegertracing/jaeger-operator/pkg/util" @@ -90,7 +91,7 @@ func getOAuthProxyContainer(jaeger *v1.Jaeger) corev1.Container { args = append(args, fmt.Sprintf("--openshift-sar=%s", *jaeger.Spec.Ingress.Openshift.SAR)) } - if len(jaeger.Spec.Ingress.Openshift.DelegateUrls) > 0 && viper.GetBool("auth-delegator-available") { + if len(jaeger.Spec.Ingress.Openshift.DelegateUrls) > 0 && autodetect.OperatorConfiguration.IsAuthDelegatorAvailable() { args = append(args, fmt.Sprintf("--openshift-delegate-urls=%s", jaeger.Spec.Ingress.Openshift.DelegateUrls)) } diff --git a/pkg/inject/oauth_proxy_test.go b/pkg/inject/oauth_proxy_test.go index 5916a2699..f37716397 100644 --- a/pkg/inject/oauth_proxy_test.go +++ b/pkg/inject/oauth_proxy_test.go @@ -131,7 +131,7 @@ func TestDoNotMountWhenNotNeeded(t *testing.T) { } func TestOAuthProxyWithCustomDelegateURLs(t *testing.T) { - viper.Set("auth-delegator-available", true) + autodetect.OperatorConfiguration.SetAuthDelegatorAvailability(autodetect.AuthDelegatorAvailabilityYes) defer viper.Reset() jaeger := v1.NewJaeger(types.NamespacedName{Name: "my-instance"}) @@ -149,7 +149,7 @@ func TestOAuthProxyWithCustomDelegateURLs(t *testing.T) { } func TestOAuthProxyWithCustomDelegateURLsWithoutProperClusterRole(t *testing.T) { - viper.Set("auth-delegator-available", false) + autodetect.OperatorConfiguration.SetAuthDelegatorAvailability(autodetect.AuthDelegatorAvailabilityNo) defer func() { viper.Reset() setDefaults()