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

Egress Lockdown: Enable and Toggle #1973

Merged
merged 1 commit into from
Feb 23, 2022

Conversation

SudoBrendan
Copy link
Collaborator

@SudoBrendan SudoBrendan commented Feb 17, 2022

Which issue this PR addresses:

Fixes https://msazure.visualstudio.com/AzureRedHatOpenShift/_workitems/edit/13137954/

What this PR does / why we need it:

This feature will enable the egress lockdown feature by default on new clusters while also allowing current clusters to be admin-upgraded with the new feature.

Test plan for issue:

  • Update unit test for default cluster features
  • Update unit test for bi-directional toggling of the feature in admin update
  • Write new e2e test for internal container registry, since that takes a path from node -> gateway -> storageaccount of the registry
  • Manual testing in int-like env for:
    • Current cluster upgrade:
      • Creating cluster w/o egress lockdown
      • Admin patch that cluster with the feature
      • Make sure all is well
    • New default behavior:
      • Creating cluster w/ default (egress lockdown)
      • Make sure all is well
    • Ensure only admins can update the Egress flag

Is there any documentation that needs to be updated for this PR?

@SudoBrendan SudoBrendan changed the title Re-enable Egress Lockdown Egress Lockdown: Enable and Toggle Feb 17, 2022
Copy link
Collaborator

@bennerv bennerv left a comment

Choose a reason for hiding this comment

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

Great work so far! Couple of nits.

test/e2e/cluster.go Outdated Show resolved Hide resolved
test/e2e/cluster.go Outdated Show resolved Hide resolved
@bennerv bennerv added next-release To be included in the next RP release rollout next-up size-medium Size medium hold Hold labels Feb 23, 2022
@bennerv bennerv marked this pull request as ready for review February 23, 2022 17:12
bennerv
bennerv previously approved these changes Feb 23, 2022
Copy link
Collaborator

@bennerv bennerv left a comment

Choose a reason for hiding this comment

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

LGTM - /hold until testing is done.

Enable egress lockdown feature by default on new clusters while also
allowing current clusters to be admin-upgraded with the new feature

Co-authored-by: Ben Vesel <[email protected]>
Copy link
Contributor

@ross-bryan ross-bryan left a comment

Choose a reason for hiding this comment

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

Nice :shipit:. Should be g2g once ci/e2e is green 🟢

@bennerv
Copy link
Collaborator

bennerv commented Feb 23, 2022

/azp run e2e

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@bennerv bennerv merged commit 3db99bb into Azure:master Feb 23, 2022
@bennerv bennerv removed the hold Hold label Feb 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
next-release To be included in the next RP release rollout next-up size-medium Size medium
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants