fix(storage): treat ConstructReference keys as literal path segments by spicysharkco · Pull Request #8195 · flyteorg/flyte · GitHub
Skip to content

fix(storage): treat ConstructReference keys as literal path segments - #8195

Open
spicysharkco wants to merge 1 commit into
flyteorg:mainfrom
spicysharkco:fix/construct-reference-literal-keys
Open

spicysharkco wants to merge 1 commit into
flyteorg:mainfrom
spicysharkco:fix/construct-reference-literal-keys

Conversation

@spicysharkco

Copy link
Copy Markdown
Contributor

Why are the changes needed?

ConstructReference parses the joined keys as a URL. In an object name, # or ? then cuts the key once the reference is split, a lone % fails the call, and a colon in the first key reads as a scheme.

A directory output with a#1.csv and a#2.csv silently uploads both to raw/x/a. 100%.csv makes the call return:

Reference is of an invalid format [s3://bucket/prefix]: parse "100%.csv": invalid URL escape "%.c"

What changes were proposed in this pull request?

The keys now form a literal relative path. The returned reference escapes such characters (a%231.csv), and a name that already holds %XX, such as a%20b.csv, was decoded before and now stays literal; no caller escapes its keys. Two ioutils error tests now use an invalid base prefix instead of a shard prefix.

How was this patch tested?

Subtests in TestUrlPathConstructor_ConstructReference split a reference built from a#b.csv, what?.txt, 100%.csv, 50%25.csv and t:1.csv and compare the key with the name. They replace the key1% error case, fail without the change and pass with it.

go test ./flytestdlib/storage/ ./flyteplugins/go/tasks/pluginmachinery/ioutils/

Labels

  • fixed: For any bug fixed.

Setup process

Screenshots

Check all the applicable boxes

  • I updated the documentation accordingly.
  • All new and existing tests passed.
  • All commits are signed-off.

Related PRs

Stack

Docs link

The nested keys were parsed as a URL, so a key with '#' or '?' lost
everything from that character on once the reference was split into a
storage key, a stray '%' made the call fail, and a colon in the first
key was read as a scheme. Build the relative path from the keys as-is.

Signed-off-by: spicysharkco <321699413+spicysharkco@users.noreply.github.com>

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant