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

test: [M3-8725] - Add cypress tests for creating LKE clusters with ACL (LKE ACL integration part 3) #11132

Merged
merged 8 commits into from
Oct 24, 2024

Conversation

coliu-akamai
Copy link
Contributor

@coliu-akamai coliu-akamai commented Oct 21, 2024

Description 📝

Adds tests for updating an LKE cluster's ACL from the summary/details page
Part 1: #10968
Part 2: #11131
Part 3: #11132

Changes 🔄

Target release date 🗓️

10/28

How to test 🧪

yarn cy:run -s "cypress/e2e/core/kubernetes/lke-create.spec.ts"

As an Author I have considered 🤔

Check all that apply

  • 👀 Doing a self review
  • ❔ Our contribution guidelines
  • 🤏 Splitting feature into small PRs
  • ➕ Adding a changeset
  • 🧪 Providing/Improving test coverage
  • 🔐 Removing all sensitive information from the code and PR description
  • 🚩 Using a feature flag to protect the release
  • 👣 Providing comprehensive reproduction steps
  • 📑 Providing or updating our documentation
  • 🕛 Scheduling a pair reviewing session
  • 📱 Providing mobile support
  • ♿ Providing accessibility support

Copy link

github-actions bot commented Oct 22, 2024

Coverage Report:
Base Coverage: 86.99%
Current Coverage: 86.99%

@coliu-akamai
Copy link
Contributor Author

coliu-akamai commented Oct 23, 2024

note - the copies are adding clutter to this test. I've created M3-8776 to clean that up and write unit tests for that instead, might combine that ticket with this one tbh! (just want to get to M3-8674 and microfrontend stuff first)

Copy link
Contributor

Choose a reason for hiding this comment

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

Question: should we be seeing the ACL flow visible in the pre-existing tests in this file? In LKE update spec we have the apl feature flag off in a beforeEach statement.

Test case: 'create an LKE cluster'
Screenshot 2024-10-23 at 12 29 48 PM

Copy link
Contributor Author

@coliu-akamai coliu-akamai Oct 23, 2024

Choose a reason for hiding this comment

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

yup so the apl feature flag controls APL, a different feature than ACL (I know, the acronyms are too similar 😆)

ACL is controlled by the account capability 'LKE Network Access Control List (IP ACL)' and there is no associated feature flag. Talmai had mentioned that the ACL capability is enabled as of last week to all customers - so ACL will be going out live to everyone on 10/28

and since every account has the ACL capability added to it, that's probably why you can see it in all the LKE tests (except the one where I specify an account without the capability)

Copy link
Contributor

Choose a reason for hiding this comment

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

😵‍💫 I had a feeling I was getting things confused between APL and ACL. Wow. Okay, thank you for clarifying!

Copy link
Contributor

@mjac0bs mjac0bs left a comment

Choose a reason for hiding this comment

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

Test coverage passes locally and coverage of cases looks good. Thanks for the follow up ticket you created. Did not see any typos here.

I do have a minor complaint about strange wording of the copy, but that's probably best for another time.
When enabled, only the IP addresses and ranges specified by you can connect to the control plane. (passive voice)
⬇️
When enabled, only the IP addresses and ranges you specify can connect to the control plane. (active voice - clearer, more direct)

@mjac0bs mjac0bs added the Add'tl Approval Needed Waiting on another approval! label Oct 23, 2024
@coliu-akamai
Copy link
Contributor Author

going to be merging this as is, and will work on M3-8776 hopefully soon 🙏
@mjac0bs will bring up your copy question to the LKE ACL team!

@coliu-akamai coliu-akamai merged commit 85edbfc into linode:develop Oct 24, 2024
23 checks passed
Copy link

cypress bot commented Oct 24, 2024

Cloud Manager E2E    Run #6728

Run Properties:  status check passed Passed #6728  •  git commit 85edbfc4c0: test: [M3-8725] - Add cypress tests for creating LKE clusters with ACL (LKE ACL ...
Project Cloud Manager E2E
Branch Review develop
Run status status check passed Passed #6728
Run duration 26m 05s
Commit git commit 85edbfc4c0: test: [M3-8725] - Add cypress tests for creating LKE clusters with ACL (LKE ACL ...
Committer Connie Liu
View all properties for this run ↗︎

Test results
Tests that failed  Failures 0
Tests that were flaky  Flaky 1
Tests that did not run due to a developer annotating a test with .skip  Pending 2
Tests that did not run due to a failure in a mocha hook  Skipped 0
Tests that passed  Passing 445
View all changes introduced in this branch ↗︎

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Add'tl Approval Needed Waiting on another approval! LKE Related to Linode Kubernetes Engine offerings Testing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants