Skip to content

Add GitHub Actions workflow for SSO Playwright tests#24980

Merged
chirag-madlani merged 5 commits intomainfrom
copilot/create-placeholder-workflow
Dec 24, 2025
Merged

Add GitHub Actions workflow for SSO Playwright tests#24980
chirag-madlani merged 5 commits intomainfrom
copilot/create-placeholder-workflow

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Dec 23, 2025

Describe your changes:

Created a dedicated GitHub Actions workflow to run SSO configuration tests in isolation using Playwright's tag filtering.

Changes:

  • Added .github/workflows/playwright-sso-tests.yml - workflow that runs tests tagged with @sso
  • Tagged SSOConfiguration.spec.ts test suite with { tag: '@sso' } for selective execution

Workflow Configuration:

  • Triggers: workflow_dispatch (manual) and pull_request_target (automatic with "safe to test" label)
  • Environment: Ubuntu latest, MySQL, Node.js 18, Playwright 1.51.1
  • Test command: npx playwright test --grep @sso
  • Security: Requires label verification, excludes draft PRs
  • Artifacts: Uploads test reports with 5-day retention

Test Coverage:
Tests SSO provider configuration for Google, Azure AD, Okta, Auth0, SAML, LDAP, AWS Cognito, and Custom OIDC including provider selection, field visibility, and client type validation.

Usage:

# Local execution
cd openmetadata-ui/src/main/resources/ui/
npx playwright test --grep @sso

# CI: Add "safe to test" label to PR or trigger manually via Actions tab

Type of change:

  • Improvement

Checklist:

  • I have read the CONTRIBUTING document.
  • My PR title is Fixes <issue-number>: <short explanation>
  • I have commented on my code, particularly in hard-to-understand areas.
  • For JSON Schema changes: I updated the migration scripts or explained why it is not needed.
  • I have added tests around the new logic.
  • For connector/ingestion changes: I updated the documentation.
Original prompt

On /, Create a placeholder workflow to run it on my PR with actual logic
It's for running sso-tests on playwright


💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

Copilot AI and others added 2 commits December 23, 2025 15:38
Co-authored-by: chirag-madlani <12962843+chirag-madlani@users.noreply.github.com>
Co-authored-by: chirag-madlani <12962843+chirag-madlani@users.noreply.github.com>
Copilot AI changed the title [WIP] Create placeholder workflow for running SSO tests Add GitHub Actions workflow for SSO Playwright tests Dec 23, 2025
Copilot AI requested a review from chirag-madlani December 23, 2025 15:43
@github-actions
Copy link
Copy Markdown
Contributor

Hi there 👋 Thanks for your contribution!

The OpenMetadata team will review the PR shortly! Once it has been labeled as safe to test, the CI workflows
will start executing and we'll be able to make sure everything is working as expected.

Let us know if you need any help!

@github-actions
Copy link
Copy Markdown
Contributor

Hi there 👋 Thanks for your contribution!

The OpenMetadata team will review the PR shortly! Once it has been labeled as safe to test, the CI workflows
will start executing and we'll be able to make sure everything is working as expected.

Let us know if you need any help!

@chirag-madlani chirag-madlani marked this pull request as ready for review December 24, 2025 12:25
@github-actions
Copy link
Copy Markdown
Contributor

Hi there 👋 Thanks for your contribution!

The OpenMetadata team will review the PR shortly! Once it has been labeled as safe to test, the CI workflows
will start executing and we'll be able to make sure everything is working as expected.

Let us know if you need any help!

@chirag-madlani chirag-madlani added the safe to test Add this label to run secure Github workflows on PRs label Dec 24, 2025
@gitar-bot
Copy link
Copy Markdown

gitar-bot Bot commented Dec 24, 2025

🔍 CI failure analysis for 65b84da: Multiple CI workflows blocked by missing 'safe to test' label requirement

Issue

Three CI workflows are failing due to a missing required security label:

  • java-checkstyle (job 58868816867)
  • validate-json-yaml (job 58868951488)
  • validate-json-yaml (job 58868919728)

Root Cause

All failures occur at the label verification step before any actual validation logic runs. The repository uses jesusvasquez333/verify-pr-label-action@v1.4.0 to enforce a security gate:

Error! This pull request does not contain any of the valid labels: ['safe to test']
Exiting with an error code

Details

Each workflow follows the same pattern:

  1. Wait for Team Label - Passes (Team Label check completed successfully)
  2. Verify PR labels - Fails (PR Add GitHub Actions workflow for SSO Playwright tests #24980 lacks "safe to test" label)
  3. ⏭️ All subsequent steps - Never execute due to early exit

The workflows are configured to use pull_request_target triggers, which run in the context of the base repository with access to secrets and write permissions. The label requirement is a security control to prevent:

  • Unauthorized code execution in CI with access to secrets
  • Malicious PRs from external contributors running arbitrary workflows
  • Supply chain attacks through CI/CD pipelines

Why This Pattern Is Standard

For repositories accepting external contributions, pull_request_target workflows require manual approval because they:

  • Run with repository secrets access
  • Execute in the target repository's context
  • Can modify repository contents
  • Have elevated permissions

The "safe to test" label acts as a manual code review checkpoint where maintainers verify the code is safe before CI execution.

Solution

The solution is straightforward: A repository maintainer must add the "safe to test" label to PR #24980.

This should be done after:

  1. Reviewing all code changes in the PR
  2. Verifying no malicious or harmful code is present
  3. Confirming the workflow changes are appropriate
  4. Ensuring the changes don't expose secrets or create security risks

Once the label is added, all three workflows will automatically re-trigger and proceed with their validation logic (YAML validation, Java checkstyle, etc.).

Code Review ⚠️ Changes requested

New SSO test workflow has configuration inconsistencies between triggers, matrix options, and documented behavior that will cause dead code paths and confusion.

⚠️ Bug: Missing 'all' option in workflow_dispatch choices causes matrix mismatch

📄 .github/workflows/playwright-sso-tests.yml:47-53

The matrix configuration at line 71 handles an 'all' option:

provider: ${{ github.event.inputs.sso_provider == 'all' && fromJSON('["google", "okta", "azure", "auth0"]') || ... }}

However, 'all' is not included in the available choices (lines 47-53). Users cannot select 'all' to run tests for all providers simultaneously.

Impact: The 'all' handling code is unreachable, making it dead code. Users who want to test all providers must trigger the workflow multiple times.

Suggested fix: Add 'all' as an option:

type: choice
options:
  - all
  - google
  - okta
  - azure
  - auth0
  - saml
  - cognito
⚠️ Bug: Missing pull_request_target trigger despite PR-related conditional steps

📄 .github/workflows/playwright-sso-tests.yml:36-41 📄 .github/workflows/playwright-sso-tests.yml:85-101

The workflow has steps that check for pull_request_target events (lines 87-101):

  • "Wait for the labeler" step with condition if: ${{ github.event_name == 'pull_request_target' }}
  • "Verify PR labels" step with condition if: ${{ github.event_name == 'pull_request_target' }}

However, the workflow only defines workflow_dispatch as a trigger (line 40). The pull_request_target trigger is not defined, so these conditional steps will never execute.

Additionally, the workflow description (lines 27-28) mentions "Pull requests with 'safe to test' label" as a trigger, but this trigger is not implemented.

Impact: The workflow cannot be triggered by pull requests as documented, and the PR verification steps are dead code.

Suggested fix: Either:

  1. Add the pull_request_target trigger:
on:
  workflow_dispatch:
    # existing config
  pull_request_target:
    types: [labeled, synchronize]
  1. Or remove the pull_request_target conditional steps if PR triggering is not intended.
More details 💡 2 suggestions
💡 Bug: Incomplete provider list in 'all' matrix vs available choices

📄 .github/workflows/playwright-sso-tests.yml:71

The 'all' option in the matrix logic only includes 4 providers:

fromJSON('["google", "okta", "azure", "auth0"]')

But the workflow_dispatch choices list 6 providers:

  • google, okta, azure, auth0, saml, cognito

Impact: If 'all' option is added (as suggested), running all tests would skip 'saml' and 'cognito' providers.

Suggested fix: Update the 'all' provider list to include all providers:

fromJSON('["google", "okta", "azure", "auth0", "saml", "cognito"]')
💡 Code Quality: Commented-out schedule trigger contradicts "Nightly" workflow name

📄 .github/workflows/playwright-sso-tests.yml:35-40

The workflow is named "SSO Authentication Tests (Nightly)" but the schedule trigger is commented out (lines 37-39):

# schedule:
  #   # Run every night at 2 AM UTC
  #   - cron: '0 2 * * *'

Impact: The workflow won't run automatically as the name suggests. This could cause confusion about when/how tests are executed.

Suggested fix: Either:

  1. Uncomment the schedule trigger to enable nightly runs:
on:
  schedule:
    - cron: '0 2 * * *'
  workflow_dispatch:
  1. Or rename the workflow to reflect manual-only triggering (e.g., "SSO Authentication Tests (Manual)")

What Works Well

Well-structured workflow with proper caching, artifact upload, and cleanup steps. Good use of environment secrets with dynamic provider-based naming pattern. Includes concurrency controls to prevent duplicate runs.

Recommendations

Consider adding the commented schedule and pull_request_target triggers to enable the full intended functionality, and synchronize the provider list between the workflow_dispatch choices and matrix 'all' option.

Tip

Comment Gitar fix CI or enable auto-apply: gitar auto-apply:on

Options

Auto-apply is off Gitar will not commit updates to this branch.
✅ Code review is on Gitar will review this change.
Display: compact Hiding non-applicable rules.

Comment with these commands to change:

Auto-apply ✅ Code review Compact
gitar auto-apply:on         
gitar code-review:off         
gitar display:verbose         

Was this helpful? React with 👍 / 👎 | This comment will update automatically (Docs)

@chirag-madlani chirag-madlani merged commit cbea0c0 into main Dec 24, 2025
30 of 34 checks passed
@chirag-madlani chirag-madlani deleted the copilot/create-placeholder-workflow branch December 24, 2025 12:46
ShaileshParmar11 pushed a commit that referenced this pull request Dec 26, 2025
* Initial plan

* Add Playwright SSO tests workflow with @sso tag filtering

Co-authored-by: chirag-madlani <12962843+chirag-madlani@users.noreply.github.com>

* Add comprehensive documentation to SSO tests workflow

Co-authored-by: chirag-madlani <12962843+chirag-madlani@users.noreply.github.com>

* update workflow

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: chirag-madlani <12962843+chirag-madlani@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

safe to test Add this label to run secure Github workflows on PRs

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants