Skip to content

Conversation

@stlaz
Copy link
Contributor

@stlaz stlaz commented Mar 23, 2021

/assign @sttts

@stlaz
Copy link
Contributor Author

stlaz commented Mar 24, 2021

/retest

1 similar comment
@stlaz
Copy link
Contributor Author

stlaz commented Mar 30, 2021

/retest


privWhoamiOnlyToken, pubWhoamiOnlyToken := exutil.GenerateOAuthTokenPair()
whoamiOnlyToken := &oauthv1.OAuthAccessToken{
ObjectMeta: metav1.ObjectMeta{Name: "whoami-token-plus-some-padding-here-to-make-the-limit-" + oc.Namespace()},
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

where is "to make the limit" proporty gone? Do we lose anything? Or this just does not matter anymore because we always hash?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the tokens have a requirement on at least 32 characters length, that's something we're keeping even though it may not make too much sense now that we're using hashed tokens

oc := exutil.NewCLI("oauth-access-token-e2e-test")
ctx := context.Background()

g.It(fmt.Sprintf("accepts classic non-prefixed access tokens"), func() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does this imply an inverse test when we switch over?

A blocker BZ would be good not to forget.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we won't be able to e2e test that since we won't be able to create such a token, but might still be worth a unit test

stlaz added 2 commits March 30, 2021 13:39
Fixes parallel test suite issues that would make the tests
fail when the old tokens format is prohibited.
@sttts
Copy link
Contributor

sttts commented Mar 30, 2021

/lgtm
/approve

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Mar 30, 2021
@openshift-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: stlaz, sttts

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details 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 Mar 30, 2021
@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@stlaz
Copy link
Contributor Author

stlaz commented Mar 31, 2021

/retest

@stlaz
Copy link
Contributor Author

stlaz commented Mar 31, 2021

/test e2e-gcp-disruptive

@stlaz
Copy link
Contributor Author

stlaz commented Mar 31, 2021

/test e2e-aws-serial

@stlaz
Copy link
Contributor Author

stlaz commented Mar 31, 2021

/test e2e-gcp-builds
didn't think this one needs to pass, it does not have the "Required" label...

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

3 similar comments
@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

12 similar comments
@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

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.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants