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.

Sign up for GitHub

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

Jump to bottom

chore: Protect pentesting user and role from sweepers #3426

Open
wants to merge 3 commits into
base: dev
Choose a base branch
from
Open

chore: Protect pentesting user and role from sweepers #3426

wants to merge 3 commits into from

Conversation

sfc-gh-jcieslak
Copy link
Collaborator

Changes

  • Adjust sweepers to ignore users and roles used for testing.

@github-actions GitHub Actions
Copy link

Integration tests cancelled for f33be25e3621e3db996ac3a9b3a4636ae8268599

sfc-gh-jmichalak
sfc-gh-jmichalak reviewed Feb 28, 2025
View reviewed changes
Copy link
Collaborator

@sfc-gh-jmichalak sfc-gh-jmichalak left a comment

Choose a reason for hiding this comment

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

  1. Looks like docs check fails
  2. Nit: this would not matter in this case, but in general we should not cancel tests on PRs with changes in the sweepers.

pkg/sdk/sweepers.go Outdated
@@ -201,7 +203,7 @@ func getRoleSweeper(client *Client, suffix string) func() error {
return fmt.Errorf("sweeping roles ended with error, err = %w", err)
}
for _, role := range roles {
if strings.HasSuffix(role.Name, suffix) && !slices.Contains([]string{"ACCOUNTADMIN", "SECURITYADMIN", "SYSADMIN", "ORGADMIN", "USERADMIN", "PUBLIC"}, role.Name) {
if strings.HasSuffix(role.Name, suffix) && !slices.Contains([]string{"ACCOUNTADMIN", "SECURITYADMIN", "SYSADMIN", "ORGADMIN", "USERADMIN", "PUBLIC", snowflakeroles.PentestingRole.Name()}, role.Name) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nit: can we use []sdk.AccountObjectIdentifier here with roles from snowflakeroles package?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I dont think so, currently the sweepers are in SDK pkg so it would be circular dep, there is a TODO for it

@github-actions GitHub Actions
Copy link

Integration tests failure for 0ea1f66c9d3dcebd7e1942eb009e381e0b4b7a6d

@github-actions GitHub Actions
Copy link

Integration tests failure for c42e2951e47176471a731e40dd327abf75fac8e7

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sfc-gh-asawicki sfc-gh-asawicki sfc-gh-asawicki approved these changes

@sfc-gh-jmichalak sfc-gh-jmichalak sfc-gh-jmichalak left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@sfc-gh-jcieslak @sfc-gh-asawicki @sfc-gh-jmichalak