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

[AAP-37476]: Validate DE Urls #1188

Merged
merged 8 commits into from
Jan 17, 2025
Merged

Conversation

zkayyali812
Copy link
Collaborator

@zkayyali812 zkayyali812 commented Jan 15, 2025

This PR adds validation to DE image URLs.
Upon checking this issue out, we only had validation on registry credentials, and no validation on DE's.

This PR ensures we have the same validation on both registry credential URLs and Decision environments.
This PR also updates the validation to ensure that if a DE URL with digest is specified, that this is a valid specification.

@zkayyali812 zkayyali812 force-pushed the zk/fix-de/scheme branch 2 times, most recently from 8c2e523 to 3320cd0 Compare January 15, 2025 18:37
@zkayyali812 zkayyali812 marked this pull request as ready for review January 15, 2025 18:39
@zkayyali812 zkayyali812 requested a review from a team as a code owner January 15, 2025 18:39
@codecov-commenter
Copy link

codecov-commenter commented Jan 15, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 93.35%. Comparing base (ce08de5) to head (5d442c0).

@@            Coverage Diff             @@
##             main    #1188      +/-   ##
==========================================
+ Coverage   93.34%   93.35%   +0.01%     
==========================================
  Files         261      261              
  Lines       14875    14908      +33     
==========================================
+ Hits        13885    13918      +33     
  Misses        990      990              
Flag Coverage Δ
unit-int-tests-3.11 93.29% <100.00%> (+0.01%) ⬆️
unit-int-tests-3.12 93.35% <100.00%> (+0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
...rc/aap_eda/api/serializers/decision_environment.py 100.00% <100.00%> (ø)
src/aap_eda/core/validators.py 88.10% <100.00%> (+0.39%) ⬆️
tests/integration/api/test_decision_environment.py 98.07% <100.00%> (+0.29%) ⬆️

Alex-Izquierdo
Alex-Izquierdo previously approved these changes Jan 16, 2025
Copy link
Collaborator

@Alex-Izquierdo Alex-Izquierdo left a comment

Choose a reason for hiding this comment

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

Looking at the spec I'm not sure if we should support as well sha512. Anycase LGTM

@zkayyali812
Copy link
Collaborator Author

@Alex-Izquierdo great note! I just pushed a commit to ensure this case is covered as well. Better sooner than later

@zkayyali812 zkayyali812 merged commit b5d1035 into ansible:main Jan 17, 2025
8 checks passed
@zkayyali812 zkayyali812 deleted the zk/fix-de/scheme branch January 17, 2025 13:31
kaiokmo pushed a commit to kaiokmo/event-driven-ansible that referenced this pull request Jan 20, 2025
The 'image_url' field is incorrect in the decision_enrivonment tests
and it does not correspond to valid OCI image/tag/naming standards.

Proper validation of the image_url field was introduced here [1], and
the tests of the collection must reflect it.

[1] ansible/eda-server#1188
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.

4 participants