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.

Sign up for GitHub

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

support swift tempURLs #37

Merged
merged 1 commit into from
May 1, 2024
Merged

support swift tempURLs #37

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 19 additions & 0 deletions client/sign_url_provider.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
package client

import (
"github.com/cloudfoundry/bosh-s3cli/config"

Check failure on line 4 in client/sign_url_provider.go

View workflow job for this annotation

GitHub Actions / lint (macos-latest) 

File is not `goimports`-ed (goimports)
"time"
)

type SignURLProvider interface {
Sign(action string, objectID string, expiration time.Duration) (string, error)
}

func NewSignURLProvider(s3BlobstoreClient S3Blobstore, s3cliConfig *config.S3Cli) (SignURLProvider, error) {
if s3cliConfig.SwiftAuthAccount != "" {
client := NewSwiftClient(s3cliConfig)
return &client, nil
} else {
return &s3BlobstoreClient, nil
}
}
46 changes: 46 additions & 0 deletions client/swift_client.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
package client

import (
"crypto/hmac"
"crypto/sha256"
"encoding/hex"
"fmt"
"strconv"
"strings"
"time"

"github.com/cloudfoundry/bosh-s3cli/config"
)

type SwiftBlobstore struct {
s3cliConfig *config.S3Cli
}

func NewSwiftClient(s3cliConfig *config.S3Cli) SwiftBlobstore {
return SwiftBlobstore{s3cliConfig: s3cliConfig}
}

func (client *SwiftBlobstore) Sign(objectID string, action string, expiration time.Duration) (string, error) {
action = strings.ToUpper(action)
switch action {
case "GET", "PUT":
return client.SignedURL(action, objectID, expiration)
default:
return "", fmt.Errorf("action not implemented: %s", action)
}
}

func (client *SwiftBlobstore) SignedURL(action string, objectID string, expiration time.Duration) (string, error) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This method should be private

path := fmt.Sprintf("/v1/%s/%s/%s", client.s3cliConfig.SwiftAuthAccount, client.s3cliConfig.BucketName, objectID)

expires := time.Now().Add(expiration).Unix()
hmacBody := action + "\n" + strconv.FormatInt(expires, 10) + "\n" + path

h := hmac.New(sha256.New, []byte(client.s3cliConfig.SwiftTempURLKey))
h.Write([]byte(hmacBody))
signature := hex.EncodeToString(h.Sum(nil))

url := fmt.Sprintf("https://%s%s?temp_url_sig=%s&temp_url_expires=%d", client.s3cliConfig.Host, path, signature, expires)

return url, nil
}
4 changes: 3 additions & 1 deletion config/config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,9 @@ type S3Cli struct {
AssumeRoleArn string `json:"assume_role_arn"`
MultipartUpload bool `json:"multipart_upload"`
UseV2SigningMethod bool
HostStyle bool `json:"host_style"`
HostStyle bool `json:"host_style"`
SwiftAuthAccount string `json:"swift_auth_account"`
SwiftTempURLKey string `json:"swift_temp_url_key"`
}

// EmptyRegion is required to allow us to use the AWS SDK against S3 compatible blobstores which do not have
Expand Down
7 changes: 6 additions & 1 deletion main.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,11 @@ func main() {
log.Fatalln(err)
}

signURLProvider, err := client.NewSignURLProvider(blobstoreClient, &s3Config)
if err != nil {
log.Fatalln(err)
}

nonFlagArgs := flag.Args()
if len(nonFlagArgs) < 2 {
log.Fatalf("Expected at least two arguments got %d\n", len(nonFlagArgs))
Expand Down Expand Up @@ -114,7 +119,7 @@ func main() {
log.Fatalf("Expiration should be in the format of a duration i.e. 1h, 60m, 3600s. Got: %s", nonFlagArgs[3])
}

signedURL, err := blobstoreClient.Sign(objectID, action, expiration)
signedURL, err := signURLProvider.Sign(objectID, action, expiration)
Comment on lines -117 to +122
Copy link
Member

Choose a reason for hiding this comment

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

I'm curious why there is a conditional in client.NewSignURLProvider() which can return different clients, rather than having client.S3Blobstore.Sign() use the contents of config.S3Cli (which it already has access to) to conditionally choose the signing method?

What I have in mind would look something like:

func (client *S3Blobstore) Sign(objectID string, action string, expiration time.Duration) (string, error) {
	if client.s3cliConfig.SwiftAuthAccount != "" {
		return client.swiftSign(objectID, action, expiration)
	}

	return client.s3Sign(objectID, action, expiration)
}

func (client *S3Blobstore) s3Sign(objectID string, action string, expiration time.Duration) (string, error) {
	// implementation
}

func (client *S3Blobstore) swiftSign(objectID string, action string, expiration time.Duration) (string, error) {
	// implementation
}

Something like this would add swift signing behavior without adding a different signing-only client, and hides the details behind client.S3Blobstore.Sign().

Copy link
Contributor

Choose a reason for hiding this comment

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

I would not mix up the S3 client code with Swift specific implementations because I wouldn't expect Swift Code in that client.
So I do prefer making it obvious like the current implementation does. But that's more a question of taste ...

Copy link
Member

Choose a reason for hiding this comment

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

I guess I was confused about the origin of Swift in this case. I thought it was an extension of S3 in some way.

I agree that the Swift code doesn't belong on the client.S3Blobstore struct.

I do think that client.New() should return a generic client which has a an interface that matches that of client.S3Blobstore and has a Sign() implementation that is similar in spirit to my example above but instead calls out to whichever internal entity supports AWS S3, or Swift when Sign() is called.

func (c *GenericS3Blobstore) Sign(objectID string, action string, expiration time.Duration) (string, error) {
	if client.s3cliConfig.SwiftAuthAccount != "" {
		return c.someEntityWhichSupportsSwift.Sign(objectID, action, expiration)
	}

	return c.someEntityWhichSupportsAwsS3.Sign(objectID, action, expiration)
}

The GenericS3Blobstore should export an interface which matches:

type Blobstore interface {
	Put(src io.ReadSeeker, dest string) error
	Delete(dest string) error
	Exists(dest string) (bool, error)
	Sign(objectID string, action string, expiration time.Duration) (string, error)
}

The code in main shouldn't know, or need to know that Sign() can have multiple implementations depending on what config was passed. The result of calling client.New() should be an entity which satisfies the interface above and performs operations like Sign() based on the config is passed to New()


if err != nil {
log.Fatalf("Failed to sign request: %s", err)
Expand Down
Loading