Skip to content

chore(e2e): rhidp 6618 e2e auditor log - #2827

Merged
openshift-merge-bot[bot] merged 33 commits into
redhat-developer:mainfrom
gustavolira:RHIDP-6618-e2e-auditor-gustavo
May 6, 2025
Merged

chore(e2e): rhidp 6618 e2e auditor log#2827
openshift-merge-bot[bot] merged 33 commits into
redhat-developer:mainfrom
gustavolira:RHIDP-6618-e2e-auditor-gustavo

Conversation

@gustavolira

Copy link
Copy Markdown
Member

Description

Please explain the changes you made here.

Which issue(s) does this PR fix

  • Fixes #?

PR acceptance criteria

Please make sure that the following steps are complete:

  • GitHub Actions are completed and successful
  • Unit Tests are updated and passing
  • E2E Tests are updated and passing
  • Documentation is updated if necessary (requirement for new features)
  • Add a screenshot if the change is UX/UI related

How to test changes / Special notes to the reviewer

@openshift-ci
openshift-ci Bot requested review from albarbaro and zdrapela April 30, 2025 16:56
@gustavolira

Copy link
Copy Markdown
Member Author

/retest

@gustavolira

Copy link
Copy Markdown
Member Author

/test e2e-tests

@github-actions

Copy link
Copy Markdown
Contributor

@github-actions

Copy link
Copy Markdown
Contributor

@github-actions

Copy link
Copy Markdown
Contributor

@dzemanov dzemanov left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thank you for refactoring the repeated test cases and fixing skip of scaffolder audit logs, looks great. I have left a couple of suggestions.

Comment thread e2e-tests/playwright/e2e/audit-log/rbac-test-utils.ts Outdated
Comment thread e2e-tests/playwright/e2e/audit-log/rbac-test-utils.ts Outdated
Comment thread e2e-tests/playwright/e2e/audit-log/rbac-test-utils.ts Outdated
Comment thread e2e-tests/playwright/e2e/audit-log/scaffold.spec.ts Outdated
Comment thread e2e-tests/playwright/e2e/audit-log/scaffold.spec.ts Outdated
Comment thread e2e-tests/playwright/e2e/audit-log/auditor-rbac.spec.ts Outdated
Comment thread e2e-tests/playwright/e2e/audit-log/auditor-rbac.spec.ts Outdated
Comment thread e2e-tests/playwright/e2e/audit-log/rbac-test-utils.ts Outdated
Comment thread e2e-tests/playwright/e2e/audit-log/rbac-test-utils.ts Outdated

@dzemanov dzemanov left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thank you for the changes. I left 2 more questions and then I think we are good to go from my side. For the scaffolder tests, should we maybe update them to the new auditor version and just skip them till we can use severityLogLevelMappings, instead of deleting them? Or there is not much value in testing fetch events?

Comment thread e2e-tests/playwright/e2e/audit-log/scaffold.spec.ts Outdated
Comment thread e2e-tests/playwright/e2e/audit-log/scaffold.spec.ts Outdated
@github-actions

github-actions Bot commented May 2, 2025

Copy link
Copy Markdown
Contributor

@github-actions

github-actions Bot commented May 2, 2025

Copy link
Copy Markdown
Contributor

@github-actions

github-actions Bot commented May 3, 2025

Copy link
Copy Markdown
Contributor

@github-actions

github-actions Bot commented May 4, 2025

Copy link
Copy Markdown
Contributor

@github-actions

github-actions Bot commented May 4, 2025

Copy link
Copy Markdown
Contributor

@github-actions

github-actions Bot commented May 4, 2025

Copy link
Copy Markdown
Contributor

@github-actions

github-actions Bot commented May 4, 2025

Copy link
Copy Markdown
Contributor

@github-actions

github-actions Bot commented May 4, 2025

Copy link
Copy Markdown
Contributor

@github-actions

github-actions Bot commented May 4, 2025

Copy link
Copy Markdown
Contributor

dzemanov added 11 commits May 4, 2025 15:51
Signed-off-by: Dominika Zemanovicova <dzemanov@redhat.com>
Signed-off-by: Dominika Zemanovicova <dzemanov@redhat.com>
Signed-off-by: Dominika Zemanovicova <dzemanov@redhat.com>
Signed-off-by: Dominika Zemanovicova <dzemanov@redhat.com>
Signed-off-by: Dominika Zemanovicova <dzemanov@redhat.com>
Signed-off-by: Dominika Zemanovicova <dzemanov@redhat.com>
Signed-off-by: Dominika Zemanovicova <dzemanov@redhat.com>
Signed-off-by: Dominika Zemanovicova <dzemanov@redhat.com>
Signed-off-by: Dominika Zemanovicova <dzemanov@redhat.com>
Signed-off-by: Dominika Zemanovicova <dzemanov@redhat.com>
Signed-off-by: Dominika Zemanovicova <dzemanov@redhat.com>
… and refactoring log utilities

Signed-off-by: Gustavo Lira <guga.java@gmail.com>
@github-actions

github-actions Bot commented May 4, 2025

Copy link
Copy Markdown
Contributor

… and refactoring log utilities

Signed-off-by: Gustavo Lira <guga.java@gmail.com>
@github-actions

github-actions Bot commented May 4, 2025

Copy link
Copy Markdown
Contributor

… and refactoring log utilities

Signed-off-by: Gustavo Lira <guga.java@gmail.com>
@github-actions

github-actions Bot commented May 4, 2025

Copy link
Copy Markdown
Contributor

Comment thread e2e-tests/playwright/e2e/audit-log/auditor-catalog.spec.ts Outdated
Comment thread e2e-tests/playwright/e2e/audit-log/auditor-catalog.spec.ts Outdated
Comment thread e2e-tests/playwright/e2e/audit-log/rbac-test-utils.ts Outdated
@gustavolira

Copy link
Copy Markdown
Member Author

Thank you for the changes. I left 2 more questions and then I think we are good to go from my side. For the scaffolder tests, should we maybe update them to the new auditor version and just skip them till we can use severityLogLevelMappings, instead of deleting them? Or there is not much value in testing fetch events?

@dzemanov In practice these logs add very little audit value because they’re tied to a read-only action. During a typical scaffold run the backend issues dozens of entity-fetch calls, so enabling them just floods the log with low-severity noise without providing extra security or compliance insight.

For that reason I’d rather drop the test instead of keeping a skipped placeholder – we’re already validating all write-path events

… and refactoring log utilities

Signed-off-by: Gustavo Lira <guga.java@gmail.com>
@github-actions

github-actions Bot commented May 5, 2025

Copy link
Copy Markdown
Contributor

… and refactoring log utilities

Signed-off-by: Gustavo Lira <guga.java@gmail.com>
@github-actions

github-actions Bot commented May 5, 2025

Copy link
Copy Markdown
Contributor

… and refactoring log utilities

Signed-off-by: Gustavo Lira <guga.java@gmail.com>
@github-actions

github-actions Bot commented May 5, 2025

Copy link
Copy Markdown
Contributor

… and refactoring log utilities

Signed-off-by: Gustavo Lira <guga.java@gmail.com>
@dzemanov

dzemanov commented May 6, 2025

Copy link
Copy Markdown
Member

/retest

@github-actions

github-actions Bot commented May 6, 2025

Copy link
Copy Markdown
Contributor

@dzemanov dzemanov left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks great! Thank you for all the changes.

@dzemanov

dzemanov commented May 6, 2025

Copy link
Copy Markdown
Member

/lgtm

@albarbaro

Copy link
Copy Markdown
Member

/lgtm
/approve

@openshift-ci

openshift-ci Bot commented May 6, 2025

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: albarbaro, dzemanov

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 openshift-ci Bot added the approved label May 6, 2025
@openshift-merge-bot
openshift-merge-bot Bot merged commit 0fc7eec into redhat-developer:main May 6, 2025
Comment thread .ibm/pipelines/utils.sh
Comment on lines +1103 to +1111
sed_inplace() {
if [[ "$OSTYPE" == "darwin"* ]]; then
# macOS
sed -i '' "$@"
else
# Linux
sed -i "$@"
fi
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Hi, I suggest using GNU sed on your Mac OS machine instead of creating a new function for it. It is generally useful to use the GNU sed instead of the default one for development work, and we rely on the GNU sed all over our scripts in the RHDH repo, RHDH operator repo, etc.

We should even add it to https://github.com/redhat-developer/rhdh/blob/main/docs/e2e-tests/README.md#prerequisites

The installation is quite simple. It is just brew install gnu-sed and then add this line: export PATH="/opt/homebrew/opt/gnu-sed/libexec/gnubin:$PATH" to your ~/.zshrc file.

Comment thread .ibm/pipelines/utils.sh
Comment on lines +1114 to +1122
to_lowercase() {
if [[ "$OSTYPE" == "darwin"* ]]; then
# macOS - using tr
echo "$1" | tr '[:upper:]' '[:lower:]'
else
# Linux - using bash parameter expansion
echo "${1,,}"
fi
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

For the MacOS (zsh) support, I'd rather simplify the condition.

From: if [[ -z "${IS_OPENSHIFT}" || "${IS_OPENSHIFT,,}" == "false" ]]; then
To: if [[ -z "${IS_OPENSHIFT}" || "${IS_OPENSHIFT}" == "false" ]]; then

As you can see at L1097, the function that sets the IS_OPENSHIFT env. variable uses lower case, and it will not change throughout the test. It will not cause any issues.

chadcrum pushed a commit to chadcrum/rhdh that referenced this pull request Aug 3, 2025
* Introduce rbac auditor tests

Signed-off-by: Dominika Zemanovicova <dzemanov@redhat.com>

* Test

Signed-off-by: Dominika Zemanovicova <dzemanov@redhat.com>

* Catch the logs

Signed-off-by: Dominika Zemanovicova <dzemanov@redhat.com>

* Test pod logs

Signed-off-by: Dominika Zemanovicova <dzemanov@redhat.com>

* Enable change of namespace

Signed-off-by: Dominika Zemanovicova <dzemanov@redhat.com>

* Update pod selector

Signed-off-by: Dominika Zemanovicova <dzemanov@redhat.com>

* Filter the logs more precisely

Signed-off-by: Dominika Zemanovicova <dzemanov@redhat.com>

* Switch to json logs

Signed-off-by: Dominika Zemanovicova <dzemanov@redhat.com>

* Fix policy-read by-query

Signed-off-by: Dominika Zemanovicova <dzemanov@redhat.com>

* Enable more precise filtering

Signed-off-by: Dominika Zemanovicova <dzemanov@redhat.com>

* Make permission-evaluation more concrete

Signed-off-by: Dominika Zemanovicova <dzemanov@redhat.com>

* Remove testing lines

Signed-off-by: Dominika Zemanovicova <dzemanov@redhat.com>

* Increase tail number

Signed-off-by: Dominika Zemanovicova <dzemanov@redhat.com>

* RHIDP-6618 Refactor RBAC tests for improved structure and readability

Signed-off-by: Gustavo Lira <guga.java@gmail.com>

* test

Signed-off-by: Gustavo Lira <guga.java@gmail.com>

* test

Signed-off-by: Gustavo Lira <guga.java@gmail.com>

* RHIDP-6618 Update scaffold tests: refine event log validation and improve test descriptions

Signed-off-by: Gustavo Lira <guga.java@gmail.com>

* RHIDP-6618 - Add shared utilities for RBAC audit-log Playwright tests

Signed-off-by: Gustavo Lira <guga.java@gmail.com>

* RHIDP-6618 - Add shared utilities for RBAC audit-log Playwright tests

Signed-off-by: Gustavo Lira <guga.java@gmail.com>

* RHIDP-6618 - Add shared utilities for RBAC audit-log Playwright tests

Signed-off-by: Gustavo Lira <guga.java@gmail.com>

* RHIDP-6618 - Add shared utilities for RBAC audit-log Playwright tests

Signed-off-by: Gustavo Lira <guga.java@gmail.com>

* RHIDP-6618 Refactor RBAC API references in tests to use RBAC_API constants

Signed-off-by: Gustavo Lira <guga.java@gmail.com>

* Refactor RBAC policy update call for improved readability

Signed-off-by: Gustavo Lira <guga.java@gmail.com>

* RHIDP-6618 Rename scaffold.spec.ts to auditor-catalog.spec.ts and update test description for clarity

Signed-off-by: Gustavo Lira <guga.java@gmail.com>

* Enhance audit log validation in Playwright tests by adding log method and refactoring log utilities

Signed-off-by: Gustavo Lira <guga.java@gmail.com>

* Enhance audit log validation in Playwright tests by adding log method and refactoring log utilities

Signed-off-by: Gustavo Lira <guga.java@gmail.com>

* Enhance audit log validation in Playwright tests by adding log method and refactoring log utilities

Signed-off-by: Gustavo Lira <guga.java@gmail.com>

* Enhance audit log validation in Playwright tests by adding log method and refactoring log utilities

Signed-off-by: Gustavo Lira <guga.java@gmail.com>

* Enhance audit log validation in Playwright tests by adding log method and refactoring log utilities

Signed-off-by: Gustavo Lira <guga.java@gmail.com>

* Enhance audit log validation in Playwright tests by adding log method and refactoring log utilities

Signed-off-by: Gustavo Lira <guga.java@gmail.com>

* Enhance audit log validation in Playwright tests by adding log method and refactoring log utilities

Signed-off-by: Gustavo Lira <guga.java@gmail.com>

* Enhance audit log validation in Playwright tests by adding log method and refactoring log utilities

Signed-off-by: Gustavo Lira <guga.java@gmail.com>

* Enhance audit log validation in Playwright tests by adding log method and refactoring log utilities

Signed-off-by: Gustavo Lira <guga.java@gmail.com>

---------

Signed-off-by: Dominika Zemanovicova <dzemanov@redhat.com>
Signed-off-by: Gustavo Lira <guga.java@gmail.com>
Co-authored-by: Dominika Zemanovicova <dzemanov@redhat.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants