Skip to content

[2.7] Fix auth test#4315

Merged
YuanTingHsieh merged 2 commits intoNVIDIA:2.7from
YuanTingHsieh:fix_ci_auth_tests
Mar 13, 2026
Merged

[2.7] Fix auth test#4315
YuanTingHsieh merged 2 commits intoNVIDIA:2.7from
YuanTingHsieh:fix_ci_auth_tests

Conversation

@YuanTingHsieh
Copy link
Collaborator

Following changes in #4301 we will generate a signature.json that CI should expect.

Description

Fix ci

Types of changes

  • Non-breaking change (fix or new feature that would not break existing functionality).
  • Breaking change (fix or new feature that would cause existing functionality to change).
  • New tests added to cover the changes.
  • Quick tests passed locally by running ./runtest.sh.
  • In-line docstrings updated.
  • Documentation updated.

Copilot AI review requested due to automatic review settings March 13, 2026 20:26
@YuanTingHsieh YuanTingHsieh added the cicd continuous integration/continuous development label Mar 13, 2026
@greptile-apps
Copy link
Contributor

greptile-apps bot commented Mar 13, 2026

Greptile Summary

This PR fixes a CI integration test failure caused by the introduction of signature.json generation in PR #4301. The SignatureBuilder now writes a signature.json file into every participant's workspace directory during provisioning, so the expected output of the ls site-1 shell command in the authorization test must include it.

  • Updated the expected data string in both success-case admin_api_response assertions inside shell_commands.yml to add signature.json between readme.txt and startup — the correct alphabetical position.
  • The permission-error assertion for admin@b.org is correctly left unchanged, as it does not check file listings.
  • The change is minimal, self-contained, and accurately reflects the new on-disk state after provisioning.

Confidence Score: 5/5

  • This PR is safe to merge — it is a one-line-per-occurrence test fixture update with no production code changes.
  • The change is a trivial test fixture update that correctly adds signature.json to the expected ls output strings. The alphabetical ordering is correct, only the success-case assertions are touched, and the fix directly corresponds to the new file generated by SignatureBuilder introduced in the referenced PR [2.7]: Sign the root folders of startup kits #4301.
  • No files require special attention.

Important Files Changed

Filename Overview
tests/integration_test/data/test_configs/authorization/shell_commands.yml Adds signature.json to the expected ls site-1 output in both success-case assertions, correctly reflecting the new file generated by the SignatureBuilder introduced in PR #4301. Placement is alphabetically correct between readme.txt and startup.

Sequence Diagram

sequenceDiagram
    participant CI as CI Test Runner
    participant Driver as NVFTestDriver
    participant AdminAPI as FLAdminAPI
    participant Site as site-1

    CI->>Driver: run_event_sequence(shell_commands.yml)
    Driver->>AdminAPI: ls_target("site-1") [as admin@a.org]
    AdminAPI->>Site: ls request
    Site-->>AdminAPI: audit.log daemon_pid.fl local log.json log.txt log_error.txt pid.fl readme.txt signature.json startup transfer
    AdminAPI-->>Driver: SUCCESS response
    Driver->>Driver: assert response == expected data (includes signature.json)

    Driver->>AdminAPI: ls_target("site-1") [as admin@b.org]
    AdminAPI-->>Driver: ERROR_AUTHORIZATION
    Driver->>Driver: assert PermissionError message

    Driver->>AdminAPI: ls_target("site-1") [super admin]
    AdminAPI->>Site: ls request
    Site-->>AdminAPI: audit.log ... signature.json startup transfer
    AdminAPI-->>Driver: SUCCESS response
    Driver->>Driver: assert response == expected data (includes signature.json)
Loading

Last reviewed commit: 249d734

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Updates the authorization integration test configuration to align expected ls output with the startup-kit signing changes introduced in #4301 (signature file now generated at kit root).

Changes:

  • Update expected shell_commands ls site-1 output to include signature.json in the directory listing.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@YuanTingHsieh YuanTingHsieh merged commit bfc2f2c into NVIDIA:2.7 Mar 13, 2026
17 of 18 checks passed
@YuanTingHsieh YuanTingHsieh deleted the fix_ci_auth_tests branch March 13, 2026 21:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cicd continuous integration/continuous development

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants