Skip to content
This repository has been archived by the owner on Oct 9, 2023. It is now read-only.

Validate user-provided entity versions are url safe #537

Merged
merged 4 commits into from
Mar 13, 2023

Conversation

katrogan
Copy link
Contributor

@katrogan katrogan commented Mar 9, 2023

TL;DR

Ensures that user-provided entity version are url safe since they are used http paths to manipulate the entities and can lead to 404s when they include slashes.

Execution names, which are also user-provided and used in http paths are already constrained to a smaller subset of allowable chars

Type

  • Bug Fix
  • Feature
  • Plugin

Are all requirements met?

  • Code completed
  • Smoke tested
  • Unit tests added
  • Code documentation added
  • Any pending items have an associated Issue

Complete description

Encountered with a user

Tracking Issue

fixes flyteorg/flyte#3427

Follow-up issue

NA

Signed-off-by: Katrina Rogan <katroganGH@gmail.com>
Signed-off-by: Katrina Rogan <katroganGH@gmail.com>
@katrogan
Copy link
Contributor Author

katrogan commented Mar 9, 2023

cc @cosmicBboy

@codecov
Copy link

codecov bot commented Mar 10, 2023

Codecov Report

Merging #537 (a040b72) into master (326ca45) will increase coverage by 1.48%.
The diff coverage is 100.00%.

❗ Current head a040b72 differs from pull request most recent head 67100a6. Consider uploading reports for the commit 67100a6 to get more accurate results

@@            Coverage Diff             @@
##           master     #537      +/-   ##
==========================================
+ Coverage   60.14%   61.62%   +1.48%     
==========================================
  Files         169      169              
  Lines       15095    12420    -2675     
==========================================
- Hits         9079     7654    -1425     
+ Misses       5215     3965    -1250     
  Partials      801      801              
Flag Coverage Δ
unittests ?

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

Impacted Files Coverage Δ
pkg/manager/impl/task_execution_manager.go 72.27% <100.00%> (+2.98%) ⬆️
pkg/manager/impl/validation/validation.go 95.62% <100.00%> (+1.46%) ⬆️

... and 153 files with indirect coverage changes

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

Signed-off-by: Katrina Rogan <katroganGH@gmail.com>
Signed-off-by: Katrina Rogan <katroganGH@gmail.com>
@katrogan katrogan merged commit b96881a into master Mar 13, 2023
@katrogan katrogan deleted the validate-version branch March 13, 2023 16:21
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] Flyteadmin should validate entity versions at registration time to ensure they're url safe
2 participants