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

ARO-6425 v20240812preview validation #3537

Closed
wants to merge 8 commits into from
Closed

ARO-6425 v20240812preview validation #3537

wants to merge 8 commits into from

Conversation

yithian
Copy link
Collaborator

@yithian yithian commented Apr 24, 2024

Which issue this PR addresses:

For ARO-6425

What this PR does / why we need it:

Adds static validation for new API fields added in v20240812preview

Test plan for issue:

Includes unit tests for the new static validation features.

Is there any documentation that needs to be updated for this PR?

No, this is just static validation to ensure inputs are as expected.

How do you know this will function as expected in production?

Unit test data has been written to match expected inputs but nonetheless this should be tested in a lower environment before being pushed to production

@yithian
Copy link
Collaborator Author

yithian commented Apr 24, 2024

@microsoft-github-policy-service agree company="Red Hat"

@yithian yithian added chainsaw Pull requests or issues owned by Team Chainsaw ready-for-review labels Apr 24, 2024
Copy link
Collaborator

@cadenmarchese cadenmarchese left a comment

Choose a reason for hiding this comment

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

The validation code looks good to me. I'm going to attach a hold label here just so we don't collide with ongoing changes in https://github.com/Azure/ARO-RP/pull/3529/files. thanks!

@cadenmarchese cadenmarchese added the hold Hold label Apr 24, 2024
Copy link
Contributor

@kimorris27 kimorris27 left a comment

Choose a reason for hiding this comment

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

Thank you for the PR! I recommended some small changes to the validation logic. I'll review again once #3529 has been merged,

@mociarain
Copy link
Collaborator

CI is broken on forks so this PR will need to be moved to a branch off the main RP repo.

@yithian
Copy link
Collaborator Author

yithian commented May 7, 2024

closing this in favor of #3563

@yithian yithian closed this May 7, 2024
@yithian yithian deleted the ARO-6425_v20240812preview_validation branch May 10, 2024 14:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
chainsaw Pull requests or issues owned by Team Chainsaw hold Hold ready-for-review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants