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

Export KeyInfo.GetSSEHeaders so users can more easily generate SSE-C headers #740

Merged
merged 1 commit into from
Jul 4, 2017

Conversation

donatello
Copy link
Member

This adds a method for users to more simply generate and set server-side-encryption headers.

// pairs. They can be set as metadata in PutObject* requests (for
// encryption) or be set as request headers in `Core.GetObject` (for
// decryption).
func (s *SSEInfo) GetSSEHeaders() map[string]string {
Copy link
Member

Choose a reason for hiding this comment

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

Lets keep it map[string][]string if we are really going to export it.

Copy link
Member Author

Choose a reason for hiding this comment

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

No, doing it this way is the cleaner interface for users when adding these headers to another existing set of headers. Compare:

// In the scenarios below, metadata is of type map[string][]string as this is the type 
// passed to put-object requests.

// 1. When header value is just a string:
key := NewSSEInfo("blahblah", "")
for k, v := range key.GetSSEHeaders() {
     metadata[k] = append(metadata[k], v)
}

// 2. When header value is a []string:
key := NewSSEInfo("blahblah", "")
for k, v := range key.GetSSEHeaders() {
     // Either we extract v[0] alone - which means implementation could have to change in future, or...
     metadata[k] = append(metadata[k], v...)
}

The point here is that there is no reason to making the return type a slice - there should be no option for user to append extra stuff for these sse-c headers.

Copy link
Member

Choose a reason for hiding this comment

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

Hmm also Go doesn't allow immutable maps. Makes sense..

Copy link
Member Author

Choose a reason for hiding this comment

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

Any other reviews?

poornas added a commit to poornas/minio-go that referenced this pull request Jul 3, 2017
@deekoder deekoder requested review from vadmeste and krisis and removed request for vadmeste July 4, 2017 00:35
@krishnasrinivas krishnasrinivas merged commit 80e4a4e into minio:master Jul 4, 2017
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