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

Add extended reservation checks in the server #428

Merged
merged 10 commits into from
Jan 24, 2025
Merged

Add extended reservation checks in the server #428

merged 10 commits into from
Jan 24, 2025

Conversation

val500
Copy link
Contributor

@val500 val500 commented Dec 10, 2024

Description

This PR adds support for extended reservation times in Testflinger Server. Certain users need longer reservation times than the maximum 6 hours, so this provides this ability for authorized clients. The server rejects jobs with higher than 6 hours of requested reservation time if they do not provide credentials or if their credentials do not provide them the authorization. The authentication flow is similar to that of job priority and restricted queues.

Resolved issues

Resolves https://warthogs.atlassian.net/browse/CERTTF-458

Documentation

Documentation was added to the docs/ folder.

Web service API changes

No new endpoints, but jobs that have longer than 6 hours requested of reservation times will be rejected without prior authorization.

Tests

Unit tests were added to test_v1_authorization.py

@val500 val500 marked this pull request as draft December 10, 2024 18:25
@plars
Copy link
Contributor

plars commented Dec 12, 2024

@val500 It looks pretty straightforward so far, but of course needs documentation. Not sure you intended it to be ready for review yet since it's not filled in and marked though.

@val500 val500 force-pushed the CERTTF-458 branch 2 times, most recently from 9cba6b7 to ad36f79 Compare December 12, 2024 22:30
@val500 val500 requested a review from plars December 12, 2024 22:36
@val500 val500 marked this pull request as ready for review December 12, 2024 22:36
@jocave jocave requested review from jocave and boukeas December 16, 2024 17:18
Copy link
Contributor

@boukeas boukeas left a comment

Choose a reason for hiding this comment

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

I've left quite a lot of comments here and there -- many of them are questions. I am not too familiar with the detail of how all this comes together and I am unaware of any previous discussions you might have had with Paul so I apologize in advance for any of the comments that might be missing the mark.

server/src/api/v1.py Show resolved Hide resolved
server/src/api/v1.py Outdated Show resolved Hide resolved
server/src/api/v1.py Show resolved Hide resolved
server/src/api/v1.py Show resolved Hide resolved
server/src/api/v1.py Outdated Show resolved Hide resolved
server/src/api/v1.py Outdated Show resolved Hide resolved
server/src/api/v1.py Outdated Show resolved Hide resolved
server/src/api/v1.py Outdated Show resolved Hide resolved
server/src/api/v1.py Outdated Show resolved Hide resolved
server/src/api/v1.py Outdated Show resolved Hide resolved
boukeas
boukeas previously approved these changes Jan 22, 2025
Copy link
Contributor

@boukeas boukeas left a comment

Choose a reason for hiding this comment

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

I think the structure reflects the logic pretty well now. I have asked for one final refinement but please check that it makes sense and that it works properly. I am approving in any case so that I don't slow you down. Thank you for all this.

server/src/api/v1.py Outdated Show resolved Hide resolved
@val500 val500 merged commit 2f192d3 into main Jan 24, 2025
6 checks passed
@val500 val500 deleted the CERTTF-458 branch January 24, 2025 15:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants