Allow users to provide tags on upload #378
Conversation
This change adds the ability for users to pass in any number of tags during upload. The tags are passed as given to EBS start-snapshot. Tags are expected to be inputted as "Key=k,Value=v" where k and v are users desired tags. The key and value delimiters were chosen to adhere as much as possible to other AWS cli tag inputs. Any restrictions on tags are expected to be handled by the underlining EBS apis.
src/bin/coldsnap/main.rs
Outdated
| let k_v: (&str, &str) = input.split_once(KEY_DELIMITER).unwrap().1.split_once(VALUE_DELIMITER).unwrap(); | ||
|
|
||
| Ok(Tag::builder().key(k_v.0).value(k_v.1).build()) |
There was a problem hiding this comment.
We should reject empty keys, since they lead to an error otherwise:
❯ cargo run -- upload --tag Key=,Value= --tag Key=A,Value=B Cargo.toml
Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.10s
Running `target/debug/coldsnap upload --tag Key=,Value= --tag Key=A,Value=B Cargo.toml`
Failed to upload snapshot: Failed to start snapshot: service error: ValidationException: Tag key cannot be null or empty.: ValidationException: Tag key cannot be null or empty.
| let k_v: (&str, &str) = input.split_once(KEY_DELIMITER).unwrap().1.split_once(VALUE_DELIMITER).unwrap(); | |
| Ok(Tag::builder().key(k_v.0).value(k_v.1).build()) | |
| let (key, val) = input.split_once(KEY_DELIMITER).unwrap().1.split_once(VALUE_DELIMITER).unwrap(); | |
| if key.is_empty() { | |
| return Err("Tag inputs must contain a non-empty key entry".to_string()); | |
| } | |
| Ok(Tag::builder().key(key).value(val).build()) |
There was a problem hiding this comment.
Nice catch, I did miss this test above. I'm fine to make the change and do we want to do any of the other validations ourselves? The error spat was from letting EBS reject tag inputs so we don't have to lock step any changes to valid inputs over time. I'm letting the duplicate key and character limit fall through as well. The duplicate key would have to be done outside of the parsing function though.
There was a problem hiding this comment.
Do we want to do any of the other validations ourselves?
I'm fine with it like this - seems unnecessary to repeat all the validations that the AWS SDK is going to perform. Even the non-empty key check is a tad excessive, but it's cheap to do and provides a better error message.
Updates tag parsing to validate non-empty keys and tests for valid/invalid tags. Additionally ran cargo fmt across previous tag changes.
Closes: #172
Description of changes:
This change adds the ability for users to pass in any number of tags during upload. The tags are passed as given to EBS start-snapshot. Tags are expected to be inputted as "Key=k,Value=v" where k and v are users desired tags. The key and value delimiters were chosen to adhere as much as possible to other AWS cli tag inputs. Any restrictions on tags are expected to be handled by the underlining EBS apis.
Testing
local testing performed with the following commands and validation of upload and metadata when expected using
aws ec2 describe-snapshots --snapshot-id <<ID>>. I have removed fields from the describe commands that aren't useful for the validation of tags.No tags provided
Single tag provided:
Multiple tags provided:
Expected failure modes:
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.