Skip to content
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

Update login templates to PatternFly 4 design #244

Merged
merged 1 commit into from Jan 23, 2020
Merged

Update login templates to PatternFly 4 design #244

merged 1 commit into from Jan 23, 2020

Conversation

rhamilto
Copy link
Member

@rhamilto rhamilto commented Jan 20, 2020

The raw templates can be viewed at:

How to test:

  1. Scale cluster-version-operator to zero pods by visiting k8s/ns/openshift-cluster-version/deployments/cluster-version-operator and using the pod donut controls to set pods to zero. Additionally, pause rollouts via Actions > Pause Rollouts.
  2. Delete the existing branding secret via oc delete secret v4-0-config-system-ocp-branding-template -n openshift-authentication
  3. Recreate the branding secret via oc create -f https://raw.githubusercontent.com/openshift/cluster-authentication-operator/<HASH>/manifests/06_branding_secret.yaml
  4. Delete existing openshift-authentication pods so they are regenerated with the new branding secret via oc delete pods --all -n openshift-authentication

oauth-openshift apps rhamilto devcluster openshift com_oauth_authorize_client_id=console-oauth-client redirect_uri=http%3A%2F%2Flocalhost%3A9000%2Fauth%2Fcallback response_type=code scope=user%3Afull state=fadafc79
oauth-openshift apps rhamilto devcluster openshift com_login_test_then=%2Foauth%2Fauthorize%3Fclient_id%3Dconsole-oauth-client%26idp%3Dtest%26redirect_uri%3Dhttp%253A%252F%252Flocalhost%253A9000%252Fauth%252Fcallback%26response_type%3Dcode%
oauth-openshift apps rhamilto devcluster openshift com_login_test_reason=user_required then=%2Foauth%2Fauthorize%3Fclient_id%3Dconsole-oauth-client%26idp%3Dtest%26redirect_uri%3Dhttp%253A%252F%252Flocalhost%253A9000%252Fauth%252Fcallback%26
oauth-openshift apps rhamilto devcluster openshift com_login_test_reason=access_denied then=%2Foauth%2Fauthorize%3Fclient_id%3Dconsole-oauth-client%26idp%3Dtest%26redirect_uri%3Dhttp%253A%252F%252Flocalhost%253A9000%252Fauth%252Fcallback%26

cc: @spadgett, @benjaminapetersen

@openshift-ci-robot openshift-ci-robot added needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Jan 20, 2020
@openshift-ci-robot openshift-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jan 20, 2020
@benjaminapetersen
Copy link

benjaminapetersen commented Jan 21, 2020

@rhamilto I think you can take that file & oc apply -f <secret-file> and it should replace whats deployed. If I run:

oc get secret -n openshift-authentication
NAME                                       TYPE                                  DATA   AGE
builder-dockercfg-rmgb2                    kubernetes.io/dockercfg               1      4d20h
builder-token-gnwst                        kubernetes.io/service-account-token   4      4d20h
builder-token-z7h8z                        kubernetes.io/service-account-token   4      4d20h
default-dockercfg-29j8n                    kubernetes.io/dockercfg               1      4d20h
default-token-5sn44                        kubernetes.io/service-account-token   4      4d20h
default-token-d59zb                        kubernetes.io/service-account-token   4      4d21h
deployer-dockercfg-zfzrt                   kubernetes.io/dockercfg               1      4d20h
deployer-token-s4jw8                       kubernetes.io/service-account-token   4      4d20h
deployer-token-vc6tc                       kubernetes.io/service-account-token   4      4d20h
oauth-openshift-dockercfg-zdpjj            kubernetes.io/dockercfg               1      4d20h
oauth-openshift-token-598h8                kubernetes.io/service-account-token   4      4d21h
oauth-openshift-token-9hjl8                kubernetes.io/service-account-token   4      4d20h
# here is your name: v4-0-config-system-ocp-branding-template
v4-0-config-system-ocp-branding-template   Opaque                                3      4d21h
v4-0-config-system-router-certs            Opaque                                1      4d20h
v4-0-config-system-serving-cert            kubernetes.io/tls                     2      4d20h
v4-0-config-system-session                 Opaque                                1      4d20h

Correction, you cannot oc apply -f as apply stores previous state on annotations, and the data in the secret is significant. You have to oc delete secret v4-0-config-system-ocp-branding-template -n openshift-authentication followed by oc create -f https://raw.githubusercontent.com/openshift/cluster-authentication-operator/5e39fab0df6ba997fa0428ea9b0738c393a80001/manifests/06_branding_secret.yaml

Step 2, oc delete pods --all -n openshift-authentication to redeploy the pods (so they update the volume mount for the secret).

When I did the above the login page became:

Screen Shot 2020-01-20 at 9 48 09 PM

Note however, I did have alternative auth providers setup via HTPSSWD and I no longer had the opportunity to select one of them.

@stlaz
Copy link
Member

stlaz commented Jan 21, 2020

@benjaminapetersen is the console-login test failing because of the new template?

@stlaz
Copy link
Member

stlaz commented Jan 21, 2020

Small correction to the testing comment:
Step 0: disable CVO managing the secret so that you don't have to try to compete with it.

I couldn't get a cluster up yet since we seem to be hitting AWS limits against, I'll try that afterwards, including the oauth-server.

The IdM selection is a must-have though, if it's not there yet, add it, please.

@rhamilto
Copy link
Member Author

@benjaminapetersen is the console-login test failing because of the new template?

Indeed it is. Tests are looking for the old PF3 login class (.login-pf) that has been changed to the new PF4 login class (.pf-c-login). Lemme update the tests in a way that will work with both the old and new login templates.

@rhamilto
Copy link
Member Author

ci/prow/e2e-aws-console-login should pass once openshift/console#4029 merges.

@stlaz
Copy link
Member

stlaz commented Jan 22, 2020

tested in my cluster with 2 IdPs, selecting an IdP worked so I think we can merge this once the tests pass.

@stlaz
Copy link
Member

stlaz commented Jan 22, 2020

/test e2e-aws-upgrade

@stlaz
Copy link
Member

stlaz commented Jan 22, 2020

/retest

1 similar comment
@rhamilto
Copy link
Member Author

/retest

@spadgett
Copy link
Member

I've confirmed the stable:console-tests image used in the last run didn't have the test changes from openshift/console#4029, which is why e2e-aws-console-login failed

@rhamilto
Copy link
Member Author

/retest

3 similar comments
@rhamilto
Copy link
Member Author

/retest

@rhamilto
Copy link
Member Author

/retest

@stlaz
Copy link
Member

stlaz commented Jan 23, 2020

/retest

@stlaz
Copy link
Member

stlaz commented Jan 23, 2020

/lgtm
installation seems to be in quite a bad shape lately, let's have /retests automatically

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Jan 23, 2020
@openshift-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: rhamilto, 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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci-robot openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jan 23, 2020
@openshift-merge-robot openshift-merge-robot merged commit 38e8bca into openshift:master Jan 23, 2020
@rhamilto rhamilto deleted the templates-pf4 branch January 23, 2020 13:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants