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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for state in cloud object storage (S3, GCS, Azure) #2455

Merged
Changes from 9 commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
42ac023
fixed compilation errors
Place1 Feb 16, 2019
9bcbd4e
refactored usage of ioutils and os in filebackend to use go-cloud ins鈥
Place1 Feb 16, 2019
7a2b216
added all cloud backends to go-cloud blob storage
Place1 Feb 16, 2019
22faa8a
fixed permalink for filebackend
Place1 Feb 16, 2019
a7ec98c
filebackend will now longer do any local FS operations if the scheme 鈥
Place1 Feb 16, 2019
255a3a7
Revert "fixed compilation errors"
Place1 Feb 16, 2019
24e5813
updated lock file
Place1 Feb 16, 2019
9ad7de6
renamed IsLocalBackendURL function to IsFileStateBackendURL, replaced鈥
Place1 Mar 3, 2019
2e91fd1
Merge branch 'master' of github.com:pulumi/pulumi into add-support-fo鈥
Place1 Mar 3, 2019
7fd83c4
using context.TODO instead of context.Background
Place1 Apr 17, 2019
e569613
pulled master
Place1 Apr 17, 2019
9e13af9
Merge branch 'master' into add-support-for-state-in-cloud-object-storage
CyrusNajmabadi Apr 24, 2019
bbb6e17
Build
CyrusNajmabadi Apr 24, 2019
d22fe67
Use new function for validating schemes.
CyrusNajmabadi Apr 24, 2019
19e3b4a
Do not hardcode schemes
CyrusNajmabadi Apr 24, 2019
17f0d74
Simplify
CyrusNajmabadi Apr 24, 2019
d6d74a9
Use new Copy function.
CyrusNajmabadi Apr 24, 2019
fe83e70
Remove code to ensure directories for local files. The go file:// pr鈥
CyrusNajmabadi Apr 24, 2019
bb4c998
Use filekey
CyrusNajmabadi Apr 24, 2019
87fd20a
Remove code to ensure directories for local files. The go file:// pr鈥
CyrusNajmabadi Apr 24, 2019
e53aca2
Use new Copy function.
CyrusNajmabadi Apr 24, 2019
7cd933e
Remove unused
CyrusNajmabadi Apr 24, 2019
2943ddb
Ensure working for local stacks
CyrusNajmabadi Apr 24, 2019
35c1a0c
Linting
CyrusNajmabadi Apr 24, 2019
d9197bc
Update changelog
CyrusNajmabadi Apr 24, 2019
ef869d9
Support ~ paths
CyrusNajmabadi Apr 24, 2019
5192a66
Collect test info
CyrusNajmabadi Apr 25, 2019
6f52674
Fix
CyrusNajmabadi Apr 25, 2019
File filter...
Filter file types
Jump to鈥
Jump to file or symbol
Failed to load files and symbols.

Always

Just for now

Large diffs are not rendered by default.

Oops, something went wrong.
@@ -86,7 +86,7 @@ func newLoginCmd() *cobra.Command {

var be backend.Backend
var err error
if filestate.IsLocalBackendURL(cloudURL) {
if filestate.IsFileStateBackendURL(cloudURL) {
be, err = filestate.Login(cmdutil.Diag(), cloudURL, "")
} else {
be, err = httpstate.Login(commandContext(), cmdutil.Diag(), cloudURL, "", displayOptions)
@@ -68,7 +68,7 @@ func newLogoutCmd() *cobra.Command {

var be backend.Backend
var err error
if filestate.IsLocalBackendURL(cloudURL) {
if filestate.IsFileStateBackendURL(cloudURL) {
be, err = filestate.New(cmdutil.Diag(), cloudURL, "")
} else {
be, err = httpstate.New(cmdutil.Diag(), cloudURL, "")
@@ -59,7 +59,7 @@ func currentBackend(opts display.Options) (backend.Backend, error) {
if err != nil {
return nil, err
}
if filestate.IsLocalBackendURL(creds.Current) {
if filestate.IsFileStateBackendURL(creds.Current) {
return filestate.New(cmdutil.Diag(), creds.Current, stackConfigFile)
}
return httpstate.Login(commandContext(), cmdutil.Diag(), creds.Current, stackConfigFile, opts)
@@ -18,13 +18,20 @@ import (
"context"
"encoding/json"
"fmt"
"io/ioutil"
"net/url"
"os"
"os/user"
"path"
"path/filepath"
"strings"
"time"

"gocloud.dev/blob"
_ "gocloud.dev/blob/azureblob" // driver for azblob://
_ "gocloud.dev/blob/fileblob" // driver for file://
_ "gocloud.dev/blob/gcsblob" // driver for gs://
_ "gocloud.dev/blob/s3blob" // driver for s3://

"github.com/pkg/errors"

"github.com/pulumi/pulumi/pkg/apitype"
@@ -44,8 +51,13 @@ import (
"github.com/pulumi/pulumi/pkg/workspace"
)

// localBackendURL is fake URL scheme we use to signal we want to use the local backend vs a cloud one.
const localBackendURLPrefix = "file://"
// supportedSchemes are the fake URL schemes that are used to signal which blob storage to use when not using http state.
var supportedSchemes = []string{
"azblob://",
"file://",
"gs://",
"s3://",
}

// Backend extends the base backend interface with specific information about local backends.
type Backend interface {
@@ -57,6 +69,7 @@ type localBackend struct {
d diag.Sink
url string
stackConfigFile string
bucket *blob.Bucket
}

type localBackendReference struct {
@@ -71,18 +84,41 @@ func (r localBackendReference) Name() tokens.QName {
return r.name
}

func IsLocalBackendURL(url string) bool {
return strings.HasPrefix(url, localBackendURLPrefix)
func IsFileStateBackendURL(url string) bool {
for _, scheme := range supportedSchemes {
This conversation was marked as resolved by CyrusNajmabadi

This comment has been minimized.

Copy link
@vangent

vangent Mar 27, 2019

FYI, I filed google/go-cloud#1695 to add a function to gocloud.dev/blob that determines if a scheme is supported; then you could replace this with something like:

u, err := url.Parse(urlstr)
if err != nil {
  return false // ? or maybe true to force the error?
}
return blob.DefaultMux().IsValidScheme(u.Scheme)

This comment has been minimized.

Copy link
@bigkraig

bigkraig Apr 22, 2019

Contributor

@Place1 looks like google/go-cloud#1695 was merged so you could swap this out to IsValidScheme

This comment has been minimized.

Copy link
@vangent

vangent Apr 22, 2019

Yup -- it's only at HEAD for now but we're planning on tagging a release later this week.

This comment has been minimized.

Copy link
@CyrusNajmabadi

CyrusNajmabadi Apr 24, 2019

Contributor

Done. Thanks!

if strings.HasPrefix(url, scheme) {
return true
}
}
return false
}

func New(d diag.Sink, url, stackConfigFile string) (Backend, error) {
if !IsLocalBackendURL(url) {
return nil, errors.Errorf("local URL %s has an illegal prefix; expected %s", url, localBackendURLPrefix)
if !IsFileStateBackendURL(url) {
return nil, errors.Errorf("local URL %s has an illegal prefix; expected one of %s", url, strings.Join(supportedSchemes, ", "))
}

if strings.HasPrefix(url, "file://") {
// For file:// backend, ensure a relative path is resolved. fileblob only supports absolute paths.
localPath, _ := filepath.Abs(strings.TrimPrefix(url, "file://"))
url = path.Join("file://", localPath)
This conversation was marked as resolved by CyrusNajmabadi

This comment has been minimized.

Copy link
@vangent

vangent Mar 27, 2019

path.Join is not what you want here; it drops duplicate slashes:

https://play.golang.org/p/lgnymhswC54

The OpenBucket example here may be helpful.

// On Unix, append the dir to "file://".
// On Windows, convert "\" to "/" and add a leading "/":
dirpath := filepath.ToSlash(dir)
if os.PathSeparator != '/' && !strings.HasPrefix(dirpath, "/") {
    dirpath = "/" + dirpath
}

b, err := blob.OpenBucket(ctx, "file://"+dirpath)
...

This comment has been minimized.

Copy link
@retr0h

retr0h Mar 28, 2019

path.Join could be used, but this is probably a cheesy way to accomplish this.

https://play.golang.org/p/Ih_7Fqm_wzz

This comment has been minimized.

Copy link
@vangent

vangent Mar 28, 2019

https://play.golang.org/p/5wbCMuJpF7W
is closer. But it doesn't do the special Windows things.

This comment has been minimized.

Copy link
@retr0h

retr0h Mar 28, 2019

oh, that's neat!

This comment has been minimized.

Copy link
@CyrusNajmabadi

CyrusNajmabadi Apr 24, 2019

Contributor

Done. Thanks!


// Ensure the directory exists for a local file:// backend.
if err := os.MkdirAll(filepath.Dir(localPath), 0700); err != nil {
return nil, errors.Wrap(err, "An IO error occurred during the current operation")
}
}

bucket, err := blob.OpenBucket(context.Background(), url)

This comment has been minimized.

Copy link
@ellismg

ellismg Mar 27, 2019

Member

Could we use context.TODO() in all of these places, so we can go back later and actually contextify all of these codepaths?

if err != nil {
return nil, errors.Wrapf(err, "unable to open bucket %s", url)
}

return &localBackend{
d: d,
url: url,
stackConfigFile: stackConfigFile,
bucket: bucket,
}, nil
}

@@ -109,23 +145,8 @@ func (b *localBackend) URL() string {
return b.url
}

func (b *localBackend) Dir() string {
path := b.url[len(localBackendURLPrefix):]
if path == "~" {
user, err := user.Current()
contract.AssertNoErrorf(err, "could not determine current user")
path = user.HomeDir
} else if path == "." {
pwd, err := os.Getwd()
contract.AssertNoErrorf(err, "could not determine current working directory")
path = pwd
}
return path
}

func (b *localBackend) StateDir() string {
dir := b.Dir()
return filepath.Join(dir, workspace.BookkeepingDir)
return workspace.BookkeepingDir
}

func (b *localBackend) ParseStackReference(stackRefName string) (backend.StackReference, error) {
@@ -401,7 +422,7 @@ func (b *localBackend) apply(ctx context.Context, kind apitype.UpdateKind, stack
fmt.Printf(
op.Opts.Display.Color.Colorize(
colors.SpecHeadline+"Permalink: "+
colors.Underline+colors.BrightBlue+"file://%s"+colors.Reset+"\n"), stack.(*localStack).Path())
colors.Underline+colors.BrightBlue+"%s%s"+colors.Reset+"\n"), b.urlScheme(), stack.(*localStack).Path())

This comment has been minimized.

Copy link
@vangent

vangent Mar 27, 2019

What kind of a link is this? Is the user expected to be able to click on it? The gocloud.dev URLs work for gocloud.dev APIs, but not in your browser. If you need the latter, you can use Bucket.SignedURL.

This comment has been minimized.

Copy link
@vangent

vangent Mar 27, 2019

And what does Path() return?

}

return changes, nil
@@ -507,19 +528,19 @@ func (b *localBackend) getLocalStacks() ([]tokens.QName, error) {
// Read the stack directory.
path := b.stackPath("")

files, err := ioutil.ReadDir(path)
if err != nil && !os.IsNotExist(err) {
return nil, errors.Errorf("could not read stacks: %v", err)
files, err := listBucket(b.bucket, path)
if err != nil {
return nil, errors.Wrap(err, "error listing stacks")
}

for _, file := range files {
// Ignore directories.
if file.IsDir() {
if file.IsDir {
continue
}

// Skip files without valid extensions (e.g., *.bak files).
stackfn := file.Name()
stackfn := objectName(file)
ext := filepath.Ext(stackfn)
if _, has := encoding.Marshalers[ext]; !has {
continue
@@ -554,3 +575,9 @@ func (b *localBackend) UpdateStackTags(ctx context.Context,
// The local backend does not currently persist tags.
return errors.New("stack tags not supported in --local mode")
}

// urlScheme returns the scheme of the storage url e.g. file:// or s3://
func (b *localBackend) urlScheme() string {
uri, _ := url.Parse(b.url)
return uri.Scheme + "://"
}
@@ -0,0 +1,78 @@
package filestate

import (
"context"
"io"
"path"

"github.com/pkg/errors"
"github.com/pulumi/pulumi/pkg/util/logging"
"gocloud.dev/blob"
)

// listBucket returns a list of all files in the bucket within a given directory. go-cloud sorts the results by key
func listBucket(bucket *blob.Bucket, dir string) ([]*blob.ListObject, error) {
bucketIter := bucket.List(&blob.ListOptions{
Delimiter: "/",
Prefix: dir + "/",
})

files := []*blob.ListObject{}

ctx := context.Background()
for {
file, err := bucketIter.Next(ctx)
if err == io.EOF {
break
}
if err != nil {
return nil, errors.Wrap(err, "could not list bucket")
}
files = append(files, file)

This comment has been minimized.

Copy link
@vangent

vangent Mar 27, 2019

Nit: would it be more clear to drop directories here rather than in the caller?

}

return files, nil
}

// objectName returns the filename of a ListObject (an object from a bucket)
func objectName(obj *blob.ListObject) string {
_, filename := path.Split(obj.Key)
return filename
}

// removeAllByPrefix deletes all objects with a given prefix (i.e. filepath)
func removeAllByPrefix(bucket *blob.Bucket, dir string) error {
files, err := listBucket(bucket, dir)
if err != nil {
return errors.Wrap(err, "unable to list bucket objects for removal")
}

for _, file := range files {
err = bucket.Delete(context.Background(), file.Key)

This comment has been minimized.

Copy link
@vangent

vangent Mar 27, 2019

See my comment above; you are not filtering out directories here.

if err != nil {
logging.V(5).Infof("error deleting object: %v (%v) skipping", file.Key, err)
}
}

return nil
}

// renameObject renames an object in a bucket. the rename requires a download/upload of the object due to a go-cloud API limitation
This conversation was marked as resolved by CyrusNajmabadi

This comment has been minimized.

Copy link
@vangent

This comment has been minimized.

Copy link
@vangent

vangent Apr 22, 2019

FYI, there's now a Copy method that you can use along with Delete.

It's only at HEAD for now, but we're planning on tagging a release sometime this week.

This comment has been minimized.

Copy link
@CyrusNajmabadi

CyrusNajmabadi Apr 24, 2019

Contributor

Done thanks!

func renameObject(bucket *blob.Bucket, source string, dest string) error {
byts, err := bucket.ReadAll(context.Background(), source)
if err != nil {
return errors.Wrap(err, "reading source object to be renamed")
}

err = bucket.WriteAll(context.Background(), dest, byts, nil)
if err != nil {
return errors.Wrapf(err, "writing destination object during rename of %s", source)
}

err = bucket.Delete(context.Background(), source)

This comment has been minimized.

Copy link
@ellismg

ellismg Mar 27, 2019

Member

Just wondering if swallowing the error here is the right thing. Logically, the rename operation failed, and so I think this should fail as well. My concern here is along the following lines: A user does an operation that requires us to rename a file. We call this function and then file is copied but the old version is not deleted. The overall operation completes correctly, but the next time things run, we see state where both the old and new file exist. This would be surprising since the previous operation seemed to pass.

For example, In some places we'll call backupTarget which bottoms out here. If the delete fails, we now have the new and old versions present. IIRC, In at least one case, we call backupTarget in order to "remove" a stack, and if the delete here fails the overall pulumi stack rm invocation would pass, but the next invocation of pulumi stack ls would list it, which would be confusing.

This comment has been minimized.

Copy link
@ellismg

ellismg Mar 27, 2019

Member

On a related note, I wonder if we should try to delete the destination file in the case where removing the older file failed. I can see pros and cons of either approach, curious on your thoughts here.

if err != nil {
logging.V(5).Infof("error deleting source object after rename: %v (%v) skipping", source, err)
}

return nil
}
Oops, something went wrong.
ProTip! Use n and p to navigate between commits in a pull request.
You can鈥檛 perform that action at this time.