Skip to content

Commit

Permalink
Bug 1989055: logins to the web console fail with custom oauth cert
Browse files Browse the repository at this point in the history
The cluster-authentication-operator was recently updated to publish
custom certs to a managed config map `oauth-serving-cert`. The console
needs to trust this new cert before logins will work propertly with
custom certs.

See openshift/cluster-authentication-operator#464

https://bugzilla.redhat.com/show_bug.cgi?id=1989055
  • Loading branch information
florkbr committed Aug 3, 2021
1 parent 35bc074 commit 8a5ee89
Show file tree
Hide file tree
Showing 11 changed files with 92 additions and 92 deletions.
2 changes: 1 addition & 1 deletion pkg/api/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ const (
OpenShiftConsoleConfigMapName = "console-config"
OpenShiftConsolePublicConfigMapName = "console-public"
ServiceCAConfigMapName = "service-ca"
DefaultIngressCertConfigMapName = "default-ingress-cert"
OAuthServingCertConfigMapName = "oauth-serving-cert"
OpenShiftConsoleDeploymentName = OpenShiftConsoleName
OpenShiftConsoleServiceName = OpenShiftConsoleName
OpenshiftConsoleRedirectServiceName = "console-redirect"
Expand Down
4 changes: 2 additions & 2 deletions pkg/console/controllers/healthcheck/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ func NewHealthCheckController(

return factory.New().
WithFilteredEventsInformers( // service
util.NamesFilter(api.TrustedCAConfigMapName, api.DefaultIngressCertConfigMapName),
util.NamesFilter(api.TrustedCAConfigMapName, api.OAuthServingCertConfigMapName),
configMapInformer.Informer(),
).WithFilteredEventsInformers( // route
util.NamesFilter(api.OpenShiftConsoleRouteName, api.OpenshiftConsoleCustomRouteName),
Expand Down Expand Up @@ -164,7 +164,7 @@ func (c *HealthCheckController) getCA(ctx context.Context, tls *routev1.TLSConfi
}
}

for _, cmName := range []string{api.TrustedCAConfigMapName, api.DefaultIngressCertConfigMapName} {
for _, cmName := range []string{api.TrustedCAConfigMapName, api.OAuthServingCertConfigMapName} {
cm, err := c.configMapClient.ConfigMaps(api.OpenShiftConsoleNamespace).Get(ctx, cmName, metav1.GetOptions{})
if err != nil {
klog.V(4).Infof("failed to GET configmap %s / %s ", api.OpenShiftConsoleNamespace, cmName)
Expand Down
4 changes: 2 additions & 2 deletions pkg/console/controllers/route/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ func NewRouteSyncController(
operatorConfigInformer.Informer(),
configV1Informers.Ingresses().Informer(),
).WithFilteredEventsInformers( // service
util.NamesFilter(api.TrustedCAConfigMapName, api.DefaultIngressCertConfigMapName),
util.NamesFilter(api.TrustedCAConfigMapName, api.OAuthServingCertConfigMapName),
configMapInformer.Informer(),
).WithInformers(
secretInformer.Informer(),
Expand Down Expand Up @@ -385,7 +385,7 @@ func (c *RouteSyncController) getCA(ctx context.Context, tls *routev1.TLSConfig)
}
}

for _, cmName := range []string{api.TrustedCAConfigMapName, api.DefaultIngressCertConfigMapName} {
for _, cmName := range []string{api.TrustedCAConfigMapName, api.OAuthServingCertConfigMapName} {
cm, err := c.configMapClient.ConfigMaps(api.OpenShiftConsoleNamespace).Get(ctx, cmName, metav1.GetOptions{})
if err != nil {
klog.V(4).Infof("failed to GET configmap %s / %s ", api.OpenShiftConsoleNamespace, cmName)
Expand Down
36 changes: 18 additions & 18 deletions pkg/console/operator/sync_v400.go
Original file line number Diff line number Diff line change
Expand Up @@ -101,10 +101,10 @@ func (co *consoleOperator) sync_v400(ctx context.Context, controllerContext fact
return statusHandler.FlushAndReturn(customLogoError)
}

defaultIngressCertConfigMap, defaultIngressCertErrReason, defaultIngressCertErr := co.ValidateDefaultIngressCertConfigMap(ctx)
statusHandler.AddConditions(status.HandleProgressingOrDegraded("DefaultIngressCertValidation", defaultIngressCertErrReason, defaultIngressCertErr))
if defaultIngressCertErr != nil {
return statusHandler.FlushAndReturn(defaultIngressCertErr)
oauthServingCertConfigMap, oauthServingCertErrReason, oauthServingCertErr := co.ValidateOAuthServingCertConfigMap(ctx)
statusHandler.AddConditions(status.HandleProgressingOrDegraded("OAuthServingCertValidation", oauthServingCertErrReason, oauthServingCertErr))
if oauthServingCertErr != nil {
return statusHandler.FlushAndReturn(oauthServingCertErr)
}

sec, secChanged, secErr := co.SyncSecret(ctx, set.Operator, controllerContext.Recorder())
Expand All @@ -121,7 +121,7 @@ func (co *consoleOperator) sync_v400(ctx context.Context, controllerContext fact
return statusHandler.FlushAndReturn(oauthErr)
}

actualDeployment, depChanged, depErrReason, depErr := co.SyncDeployment(ctx, set.Operator, cm, serviceCAConfigMap, defaultIngressCertConfigMap, trustedCAConfigMap, sec, set.Proxy, set.Infrastructure, customLogoCanMount, controllerContext.Recorder())
actualDeployment, depChanged, depErrReason, depErr := co.SyncDeployment(ctx, set.Operator, cm, serviceCAConfigMap, oauthServingCertConfigMap, trustedCAConfigMap, sec, set.Proxy, set.Infrastructure, customLogoCanMount, controllerContext.Recorder())
toUpdate = toUpdate || depChanged
statusHandler.AddConditions(status.HandleProgressingOrDegraded("DeploymentSync", depErrReason, depErr))
if depErr != nil {
Expand Down Expand Up @@ -237,7 +237,7 @@ func (co *consoleOperator) SyncDeployment(
operatorConfig *operatorv1.Console,
cm *corev1.ConfigMap,
serviceCAConfigMap *corev1.ConfigMap,
defaultIngressCertConfigMap *corev1.ConfigMap,
oauthServingCertConfigMap *corev1.ConfigMap,
trustedCAConfigMap *corev1.ConfigMap,
sec *corev1.Secret,
proxyConfig *configv1.Proxy,
Expand All @@ -246,7 +246,7 @@ func (co *consoleOperator) SyncDeployment(
recorder events.Recorder) (consoleDeployment *appsv1.Deployment, changed bool, reason string, err error) {

updatedOperatorConfig := operatorConfig.DeepCopy()
requiredDeployment := deploymentsub.DefaultDeployment(operatorConfig, cm, serviceCAConfigMap, defaultIngressCertConfigMap, trustedCAConfigMap, sec, proxyConfig, infrastructureConfig, canMountCustomLogo)
requiredDeployment := deploymentsub.DefaultDeployment(operatorConfig, cm, serviceCAConfigMap, oauthServingCertConfigMap, trustedCAConfigMap, sec, proxyConfig, infrastructureConfig, canMountCustomLogo)
genChanged := operatorConfig.ObjectMeta.Generation != operatorConfig.Status.ObservedGeneration

if genChanged {
Expand Down Expand Up @@ -319,11 +319,11 @@ func (co *consoleOperator) SyncConfigMap(
}

useDefaultCAFile := false
// We are syncing the `default-ingress-cert` configmap from `openshift-config-managed` to `openshift-console`.
// `default-ingress-cert` is only published in `openshift-config-managed` in OpenShift 4.4.0 and newer.
// If the `default-ingress-cert` configmap in `openshift-console` exists, we should mount that to the console container,
// We are syncing the `oauth-serving-cert` configmap from `openshift-config-managed` to `openshift-console`.
// `oauth-serving-cert` is only published in `openshift-config-managed` in OpenShift 4.9.0 and newer.
// If the `oauth-serving-cert` configmap in `openshift-console` exists, we should mount that to the console container,
// otherwise default to `/var/run/secrets/kubernetes.io/serviceaccount/ca.crt`
_, rcaErr := co.configMapClient.ConfigMaps(api.OpenShiftConsoleNamespace).Get(ctx, api.DefaultIngressCertConfigMapName, metav1.GetOptions{})
_, rcaErr := co.configMapClient.ConfigMaps(api.OpenShiftConsoleNamespace).Get(ctx, api.OAuthServingCertConfigMapName, metav1.GetOptions{})
if rcaErr != nil && apierrors.IsNotFound(rcaErr) {
useDefaultCAFile = true
}
Expand Down Expand Up @@ -434,18 +434,18 @@ func (co *consoleOperator) SyncCustomLogoConfigMap(ctx context.Context, operator
return okToMount, reason, err
}

func (co *consoleOperator) ValidateDefaultIngressCertConfigMap(ctx context.Context) (defaultIngressCert *corev1.ConfigMap, reason string, err error) {
defaultIngressCertConfigMap, err := co.configMapClient.ConfigMaps(api.OpenShiftConsoleNamespace).Get(ctx, api.DefaultIngressCertConfigMapName, metav1.GetOptions{})
func (co *consoleOperator) ValidateOAuthServingCertConfigMap(ctx context.Context) (oauthServingCert *corev1.ConfigMap, reason string, err error) {
oauthServingCertConfigMap, err := co.configMapClient.ConfigMaps(api.OpenShiftConsoleNamespace).Get(ctx, api.OAuthServingCertConfigMapName, metav1.GetOptions{})
if err != nil {
klog.V(4).Infoln("default-ingress-cert configmap not found")
return nil, "FailedGet", fmt.Errorf("default-ingress-cert configmap not found")
klog.V(4).Infoln("oauth-serving-cert configmap not found")
return nil, "FailedGet", fmt.Errorf("oauth-serving-cert configmap not found")
}

_, caBundle := defaultIngressCertConfigMap.Data["ca-bundle.crt"]
_, caBundle := oauthServingCertConfigMap.Data["ca-bundle.crt"]
if !caBundle {
return nil, "MissingDefaultIngressCertBundle", fmt.Errorf("default-ingress-cert configmap is missing ca-bundle.crt data")
return nil, "MissingOAuthServingCertBundle", fmt.Errorf("oauth-serving-cert configmap is missing ca-bundle.crt data")
}
return defaultIngressCertConfigMap, "", nil
return oauthServingCertConfigMap, "", nil
}

// on each pass of the operator sync loop, we need to check the
Expand Down
6 changes: 3 additions & 3 deletions pkg/console/starter/starter.go
Original file line number Diff line number Diff line change
Expand Up @@ -400,12 +400,12 @@ func RunOperator(ctx context.Context, controllerContext *controllercmd.Controlle

// startResourceSyncing should start syncing process of all secrets and configmaps that need to be synced.
func startStaticResourceSyncing(resourceSyncer *resourcesynccontroller.ResourceSyncController) error {
// sync: 'default-ingress-cert' configmap
// sync: 'oauth-serving-cert' configmap
// from: 'openshift-config-managed' namespace
// to: 'openshift-console' namespace
return resourceSyncer.SyncConfigMap(
resourcesynccontroller.ResourceLocation{Name: api.DefaultIngressCertConfigMapName, Namespace: api.OpenShiftConsoleNamespace},
resourcesynccontroller.ResourceLocation{Name: api.DefaultIngressCertConfigMapName, Namespace: api.OpenShiftConfigManagedNamespace},
resourcesynccontroller.ResourceLocation{Name: api.OAuthServingCertConfigMapName, Namespace: api.OpenShiftConsoleNamespace},
resourcesynccontroller.ResourceLocation{Name: api.OAuthServingCertConfigMapName, Namespace: api.OpenShiftConfigManagedNamespace},
)
}

Expand Down
4 changes: 2 additions & 2 deletions pkg/console/subresource/configmap/configmap.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ func DefaultConfigMap(
LogoutURL(defaultLogoutURL).
Brand(DEFAULT_BRAND).
DocURL(DEFAULT_DOC_URL).
DefaultIngressCert(useDefaultCAFile).
OAuthServingCert(useDefaultCAFile).
APIServerURL(getApiUrl(infrastructureConfig)).
InactivityTimeout(inactivityTimeoutSeconds).
ConfigYAML()
Expand All @@ -65,7 +65,7 @@ func DefaultConfigMap(
LogoutURL(consoleConfig.Spec.Authentication.LogoutRedirect).
Brand(operatorConfig.Spec.Customization.Brand).
DocURL(operatorConfig.Spec.Customization.DocumentationBaseURL).
DefaultIngressCert(useDefaultCAFile).
OAuthServingCert(useDefaultCAFile).
APIServerURL(getApiUrl(infrastructureConfig)).
Plugins(pluginsEndpoingMap).
CustomLogoFile(operatorConfig.Spec.Customization.CustomLogoFile.Key).
Expand Down
6 changes: 3 additions & 3 deletions pkg/console/subresource/configmap/configmap_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ providers: {}
},
},
{
name: "Test configmap with default-ingress-cert",
name: "Test configmap with oauth-serving-cert",
args: args{
operatorConfig: &operatorv1.Console{},
consoleConfig: &configv1.Console{},
Expand Down Expand Up @@ -128,7 +128,7 @@ apiVersion: console.openshift.io/v1
auth:
clientID: console
clientSecretFile: /var/oauth-config/clientSecret
oauthEndpointCAFile: /var/default-ingress-cert/ca-bundle.crt
oauthEndpointCAFile: /var/oauth-serving-cert/ca-bundle.crt
clusterInfo:
consoleBaseAddress: https://` + host + `
masterPublicURL: ` + mockAPIServer + `
Expand Down Expand Up @@ -456,7 +456,7 @@ apiVersion: console.openshift.io/v1
auth:
clientID: console
clientSecretFile: /var/oauth-config/clientSecret
oauthEndpointCAFile: /var/default-ingress-cert/ca-bundle.crt
oauthEndpointCAFile: /var/oauth-serving-cert/ca-bundle.crt
clusterInfo:
consoleBaseAddress: https://` + customHostname + `
masterPublicURL: ` + mockAPIServer + `
Expand Down
12 changes: 6 additions & 6 deletions pkg/console/subresource/consoleserver/config_builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,9 @@ import (
)

const (
clientSecretFilePath = "/var/oauth-config/clientSecret"
defaultIngressCertFilePath = "/var/default-ingress-cert/ca-bundle.crt"
oauthEndpointCAFilePath = "/var/run/secrets/kubernetes.io/serviceaccount/ca.crt"
clientSecretFilePath = "/var/oauth-config/clientSecret"
oauthServingCertFilePath = "/var/oauth-serving-cert/ca-bundle.crt"
oauthEndpointCAFilePath = "/var/run/secrets/kubernetes.io/serviceaccount/ca.crt"
// serving info
certFilePath = "/var/serving-cert/tls.crt"
keyFilePath = "/var/serving-cert/tls.key"
Expand Down Expand Up @@ -107,12 +107,12 @@ func (b *ConsoleServerCLIConfigBuilder) StatusPageID(id string) *ConsoleServerCL
return b
}

func (b *ConsoleServerCLIConfigBuilder) DefaultIngressCert(useDefaultDefaultIngressCert bool) *ConsoleServerCLIConfigBuilder {
if useDefaultDefaultIngressCert {
func (b *ConsoleServerCLIConfigBuilder) OAuthServingCert(useDefaultCAFile bool) *ConsoleServerCLIConfigBuilder {
if useDefaultCAFile {
b.CAFile = oauthEndpointCAFilePath
return b
}
b.CAFile = defaultIngressCertFilePath
b.CAFile = oauthServingCertFilePath
return b
}

Expand Down
14 changes: 7 additions & 7 deletions pkg/console/subresource/consoleserver/config_builder_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ func TestConsoleServerCLIConfigBuilder(t *testing.T) {
APIServerURL("https://foobar.com/api").
Host("https://foobar.com/host").
LogoutURL("https://foobar.com/logout").
DefaultIngressCert(false).
OAuthServingCert(false).
Config()
},
output: Config{
Expand All @@ -69,7 +69,7 @@ func TestConsoleServerCLIConfigBuilder(t *testing.T) {
Auth: Auth{
ClientID: api.OpenShiftConsoleName,
ClientSecretFile: clientSecretFilePath,
OAuthEndpointCAFile: defaultIngressCertFilePath,
OAuthEndpointCAFile: oauthServingCertFilePath,
LogoutRedirect: "https://foobar.com/logout",
},
Customization: Customization{},
Expand All @@ -83,7 +83,7 @@ func TestConsoleServerCLIConfigBuilder(t *testing.T) {
APIServerURL("https://foobar.com/api").
Host("https://foobar.com/host").
LogoutURL("https://foobar.com/logout").
DefaultIngressCert(false).
OAuthServingCert(false).
Config()
},
output: Config{
Expand All @@ -102,7 +102,7 @@ func TestConsoleServerCLIConfigBuilder(t *testing.T) {
Auth: Auth{
ClientID: api.OpenShiftConsoleName,
ClientSecretFile: clientSecretFilePath,
OAuthEndpointCAFile: defaultIngressCertFilePath,
OAuthEndpointCAFile: oauthServingCertFilePath,
LogoutRedirect: "https://foobar.com/logout",
},
Customization: Customization{},
Expand Down Expand Up @@ -435,7 +435,7 @@ providers: {}
APIServerURL("https://foobar.com/api").
Host("https://foobar.com/host").
LogoutURL("https://foobar.com/logout").
DefaultIngressCert(false).
OAuthServingCert(false).
ConfigYAML()
},
output: `apiVersion: console.openshift.io/v1
Expand All @@ -450,7 +450,7 @@ clusterInfo:
auth:
clientID: console
clientSecretFile: /var/oauth-config/clientSecret
oauthEndpointCAFile: /var/default-ingress-cert/ca-bundle.crt
oauthEndpointCAFile: /var/oauth-serving-cert/ca-bundle.crt
logoutRedirect: https://foobar.com/logout
customization: {}
providers: {}
Expand Down Expand Up @@ -632,7 +632,7 @@ providers: {}
"plugin1": "plugin1_url",
"plugin2": "plugin2_url",
}).
DefaultIngressCert(true)
OAuthServingCert(true)
return b.ConfigYAML()
},
output: `apiVersion: console.openshift.io/v1
Expand Down
44 changes: 22 additions & 22 deletions pkg/console/subresource/deployment/deployment.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,16 +27,16 @@ const (
)

const (
configMapResourceVersionAnnotation = "console.openshift.io/console-config-version"
proxyConfigResourceVersionAnnotation = "console.openshift.io/proxy-config-version"
infrastructureConfigResourceVersionAnnotation = "console.openshift.io/infrastructure-config-version"
serviceCAConfigMapResourceVersionAnnotation = "console.openshift.io/service-ca-config-version"
defaultIngressCertConfigMapResourceVersionAnnotation = "console.openshift.io/default-ingress-cert-config-version"
trustedCAConfigMapResourceVersionAnnotation = "console.openshift.io/trusted-ca-config-version"
secretResourceVersionAnnotation = "console.openshift.io/oauth-secret-version"
consoleImageAnnotation = "console.openshift.io/image"
workloadManagementAnnotation = "target.workload.openshift.io/management"
workloadManagementAnnotationValue = `{"effect": "PreferredDuringScheduling"}`
configMapResourceVersionAnnotation = "console.openshift.io/console-config-version"
proxyConfigResourceVersionAnnotation = "console.openshift.io/proxy-config-version"
infrastructureConfigResourceVersionAnnotation = "console.openshift.io/infrastructure-config-version"
serviceCAConfigMapResourceVersionAnnotation = "console.openshift.io/service-ca-config-version"
oauthServingCertConfigMapResourceVersionAnnotation = "console.openshift.io/oauth-serving-cert-config-version"
trustedCAConfigMapResourceVersionAnnotation = "console.openshift.io/trusted-ca-config-version"
secretResourceVersionAnnotation = "console.openshift.io/oauth-secret-version"
consoleImageAnnotation = "console.openshift.io/image"
workloadManagementAnnotation = "target.workload.openshift.io/management"
workloadManagementAnnotationValue = `{"effect": "PreferredDuringScheduling"}`
)

var (
Expand All @@ -45,7 +45,7 @@ var (
proxyConfigResourceVersionAnnotation,
infrastructureConfigResourceVersionAnnotation,
serviceCAConfigMapResourceVersionAnnotation,
defaultIngressCertConfigMapResourceVersionAnnotation,
oauthServingCertConfigMapResourceVersionAnnotation,
trustedCAConfigMapResourceVersionAnnotation,
secretResourceVersionAnnotation,
consoleImageAnnotation,
Expand All @@ -63,19 +63,19 @@ type volumeConfig struct {
mappedKeys map[string]string
}

func DefaultDeployment(operatorConfig *operatorv1.Console, cm *corev1.ConfigMap, serviceCAConfigMap *corev1.ConfigMap, defaultIngressCertConfigMap *corev1.ConfigMap, trustedCAConfigMap *corev1.ConfigMap, sec *corev1.Secret, proxyConfig *configv1.Proxy, infrastructureConfig *configv1.Infrastructure, canMountCustomLogo bool) *appsv1.Deployment {
func DefaultDeployment(operatorConfig *operatorv1.Console, cm *corev1.ConfigMap, serviceCAConfigMap *corev1.ConfigMap, oauthServingCertConfigMap *corev1.ConfigMap, trustedCAConfigMap *corev1.ConfigMap, sec *corev1.Secret, proxyConfig *configv1.Proxy, infrastructureConfig *configv1.Infrastructure, canMountCustomLogo bool) *appsv1.Deployment {
labels := util.LabelsForConsole()
meta := util.SharedMeta()
meta.Labels = labels
deploymentAnnotations := map[string]string{
configMapResourceVersionAnnotation: cm.GetResourceVersion(),
serviceCAConfigMapResourceVersionAnnotation: serviceCAConfigMap.GetResourceVersion(),
defaultIngressCertConfigMapResourceVersionAnnotation: defaultIngressCertConfigMap.GetResourceVersion(),
trustedCAConfigMapResourceVersionAnnotation: trustedCAConfigMap.GetResourceVersion(),
proxyConfigResourceVersionAnnotation: proxyConfig.GetResourceVersion(),
infrastructureConfigResourceVersionAnnotation: infrastructureConfig.GetResourceVersion(),
secretResourceVersionAnnotation: sec.GetResourceVersion(),
consoleImageAnnotation: util.GetImageEnv("CONSOLE_IMAGE"),
configMapResourceVersionAnnotation: cm.GetResourceVersion(),
serviceCAConfigMapResourceVersionAnnotation: serviceCAConfigMap.GetResourceVersion(),
oauthServingCertConfigMapResourceVersionAnnotation: oauthServingCertConfigMap.GetResourceVersion(),
trustedCAConfigMapResourceVersionAnnotation: trustedCAConfigMap.GetResourceVersion(),
proxyConfigResourceVersionAnnotation: proxyConfig.GetResourceVersion(),
infrastructureConfigResourceVersionAnnotation: infrastructureConfig.GetResourceVersion(),
secretResourceVersionAnnotation: sec.GetResourceVersion(),
consoleImageAnnotation: util.GetImageEnv("CONSOLE_IMAGE"),
}

// Set any annotations as needed so that `ApplyDeployment` rolls out a
Expand Down Expand Up @@ -590,9 +590,9 @@ func defaultVolumeConfig() []volumeConfig {
isConfigMap: true,
},
{
name: api.DefaultIngressCertConfigMapName,
name: api.OAuthServingCertConfigMapName,
readOnly: true,
path: "/var/default-ingress-cert",
path: "/var/oauth-serving-cert",
isConfigMap: true,
},
}
Expand Down

0 comments on commit 8a5ee89

Please sign in to comment.