-
Notifications
You must be signed in to change notification settings - Fork 91
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Observe publicHostname from top level console config #50
Observe publicHostname from top level console config #50
Conversation
Signed-off-by: Monis Khan <mkhan@redhat.com>
Signed-off-by: Monis Khan <mkhan@redhat.com>
This value is used as a valid redirect URI for the logout page. Signed-off-by: Monis Khan <mkhan@redhat.com>
This is a second PR containing the linked libs bump along with library-go related fixes (#36). Unless either is merged today, those changes should be moved into a separate PR. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Only have general comments but the PR itself is fine.
informers.WithNamespace(userConfigNamespace), | ||
), | ||
} | ||
resourceSyncerInformers := v1helpers.NewKubeInformersForNamespaces(kubeClient, targetName, userConfigNamespace) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Proposal to discussion (irrelevant to this PR): should we extend this with some kind of .WithResyncPeriod()
method?
@@ -31,15 +31,21 @@ func init() { | |||
utilruntime.Must(kubecontrolplanev1.Install(kubeControlplaneScheme)) | |||
} | |||
|
|||
func (c *authOperator) handleOAuthConfig(operatorConfig *authv1alpha1.AuthenticationOperatorConfig, route *routev1.Route, service *corev1.Service) (*configv1.OAuth, *corev1.ConfigMap, []idpSyncData, error) { | |||
func (c *authOperator) handleOAuthConfig(operatorConfig *authv1alpha1.AuthenticationOperatorConfig, route *routev1.Route, service *corev1.Service) (*configv1.OAuth, *configv1.Console, *corev1.ConfigMap, []idpSyncData, error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is starting to get a little bit silly, some of the return values should probably just become input instead.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, was just my first pass at it.
/lgtm |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: enj, stlaz The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Bump glide.yaml to 1.12.5
Signed-off-by: Monis Khan mkhan@redhat.com
bump(*)
Signed-off-by: Monis Khan mkhan@redhat.com
no-op compile fixes related to bump
Signed-off-by: Monis Khan mkhan@redhat.com
Observe publicHostname from top level console config
This value is used as a valid redirect URI for the logout page.
Signed-off-by: Monis Khan mkhan@redhat.com
@spadgett @benjaminapetersen