Rework asset identity and exposure of old assets. (#548)

Note: for the purposes of this discussion, archives will be treated as
assets, as their differences are not particularly meaningful.

Currently, the identity of an asset is derived from the hash and the
location of its contents (i.e. two assets are equal iff their contents
have the same hash and the same path/URI/inline value). This means that
changing the source of an asset will cause the engine to detect a
difference in the asset even if the source's contents are identical. At
best, this leads to inefficiencies such as unnecessary updates. This
commit changes asset identity so that it is derived solely from an
asset's hash. The source of an asset's contents is no longer part of
the asset's identity, and need only be provided if the contents
themselves may need to be available (e.g. if a hash does not yet exist
for the asset or if the asset's contents might be needed for an update).

This commit also changes the way old assets are exposed to providers.
Currently, an old asset is exposed as both its hash and its contents.
This allows providers to take a dependency on the contents of an old
asset being available, even though this is not an invariant of the
system. These changes remove the contents of old assets from their
serialized form when they are passed to providers, eliminating the
ability of a provider to take such a dependency. In combination with the
changes to asset identity, this allows a provider to detect changes to
an asset simply by comparing its old and new hashes.

This is half of the fix for [pulumi/pulumi-cloud#158]. The other half
involves changes in [pulumi/pulumi-terraform].
This commit is contained in:
Pat Gavlin 2017-11-12 11:45:13 -08:00 committed by GitHub
parent eec99c3ca9
commit 28579eba94
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
5 changed files with 132 additions and 64 deletions

View file

@ -104,14 +104,24 @@ func (a *Asset) GetURIURL() (*url.URL, bool, error) {
return nil, false, nil
}
// Equals returns true if a is value-equal to other.
// Equals returns true if a is value-equal to other. In this case, value equality is determined only by the hash: even if the contents of
// two assets come from different sources, they are treated as equal if their hashes match. Similarly, if the contents of two assets
// come from the same source but the assets have different hashes, the assets are not equal.
func (a *Asset) Equals(other *Asset) bool {
if a == nil {
return other == nil
} else if other == nil {
return false
}
return a.Hash == other.Hash && a.Text == other.Text && a.Path == other.Path && a.URI == other.URI
// If we can't get a hash for both assets, treat them as differing.
if err := a.EnsureHash(); err != nil {
return false
}
if err := other.EnsureHash(); err != nil {
return false
}
return a.Hash == other.Hash
}
// Serialize returns a weakly typed map that contains the right signature for serialization purposes.
@ -158,15 +168,32 @@ func DeserializeAsset(obj map[string]interface{}) (*Asset, bool, error) {
if v, has := obj[AssetURIProperty]; has {
uri = v.(string)
}
if text == "" && path == "" && uri == "" {
return &Asset{}, false, errors.New("asset is missing one of text, path, or URI")
}
return &Asset{Hash: hash, Text: text, Path: path, URI: uri}, true, nil
}
// HasContents indicates whether or not an asset's contents can be read.
func (a *Asset) HasContents() bool {
return a.IsText() || a.IsPath() || a.IsURI()
}
// Bytes returns the contents of the asset as a byte slice.
func (a *Asset) Bytes() ([]byte, error) {
// If this is a text asset, just return its bytes directly.
if text, istext := a.GetText(); istext {
return []byte(text), nil
}
blob, err := a.Read()
if err != nil {
return nil, err
}
return ioutil.ReadAll(blob)
}
// Read begins reading an asset.
func (a *Asset) Read() (*Blob, error) {
contract.Assertf(a.HasContents(), "cannot read an asset that has no contents")
if a.IsText() {
return a.readText()
} else if a.IsPath() {
@ -174,7 +201,6 @@ func (a *Asset) Read() (*Blob, error) {
} else if a.IsURI() {
return a.readURI()
}
contract.Failf("Invalid asset; one of Text, Path, or URI must be non-nil")
return nil, nil
}
@ -389,47 +415,24 @@ func (a *Archive) GetURIURL() (*url.URL, bool, error) {
return nil, false, nil
}
// Equals returns true if a is value-equal to other.
// Equals returns true if a is value-equal to other. In this case, value equality is determined only by the hash: even if the contents of
// two archives come from different sources, they are treated as equal if their hashes match. Similarly, if the contents of two archives
// come from the same source but the archives have different hashes, the archives are not equal.
func (a *Archive) Equals(other *Archive) bool {
if a == nil {
return other == nil
} else if other == nil {
return false
}
if a.Assets != nil {
if other.Assets == nil {
return false
}
if len(a.Assets) != len(other.Assets) {
return false
}
for key, value := range a.Assets {
otherv := other.Assets[key]
switch valuet := value.(type) {
case *Asset:
if othera, isAsset := otherv.(*Asset); isAsset {
if !valuet.Equals(othera) {
return false
}
} else {
return false
}
case *Archive:
if othera, isArchive := otherv.(*Archive); isArchive {
if !valuet.Equals(othera) {
return false
}
} else {
return false
}
default:
return false
}
}
} else if other.Assets != nil {
// If we can't get a hash for both archives, treat them as differing.
if err := a.EnsureHash(); err != nil {
return false
}
return a.Hash == other.Hash && a.Path == other.Path && a.URI == other.URI
if err := other.EnsureHash(); err != nil {
return false
}
return a.Hash == other.Hash
}
// Serialize returns a weakly typed map that contains the right signature for serialization purposes.
@ -514,13 +517,15 @@ func DeserializeArchive(obj map[string]interface{}) (*Archive, bool, error) {
if v, has := obj[ArchiveURIProperty]; has {
uri = v.(string)
}
if assets == nil && path == "" && uri == "" {
return &Archive{}, false, errors.New("archive is missing one of assets, path, or URI")
}
return &Archive{Hash: hash, Assets: assets, Path: path, URI: uri}, true, nil
}
// HasContents indicates whether or not an archive's contents can be read.
func (a *Archive) HasContents() bool {
return a.IsAssets() || a.IsPath() || a.IsURI()
}
// ArchiveReader presents the contents of an archive as a stream of named blobs.
type ArchiveReader interface {
// Next returns the name and contents of the next member of the archive. If there are no more members in the archive, this function
@ -533,6 +538,7 @@ type ArchiveReader interface {
// Open returns an ArchiveReader that can be used to iterate over the named blobs that comprise the archive.
func (a *Archive) Open() (ArchiveReader, error) {
contract.Assertf(a.HasContents(), "cannot read an archive that has no contents")
if a.IsAssets() {
return a.readAssets()
} else if a.IsPath() {
@ -540,7 +546,6 @@ func (a *Archive) Open() (ArchiveReader, error) {
} else if a.IsURI() {
return a.readURI()
}
contract.Failf("Invalid archive; one of Assets, Path, or URI must be non-nil")
return nil, nil
}

View file

@ -113,7 +113,7 @@ func TestAssetSerialize(t *testing.T) {
assert.Equal(t, "23f6c195eb154be262216cd97209f2dcc8a40038ac8ec18ca6218d3e3dfacd4e", archDes.Hash)
}
{
file, err := tempArchive("test")
file, err := tempArchive("test", false)
assert.Nil(t, err)
defer func() { contract.IgnoreError(os.Remove(file)) }()
arch, err := NewPathArchive(file)
@ -128,7 +128,7 @@ func TestAssetSerialize(t *testing.T) {
assert.Equal(t, "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855", archDes.Hash)
}
{
file, err := tempArchive("test")
file, err := tempArchive("test", false)
assert.Nil(t, err)
defer func() { contract.IgnoreError(os.Remove(file)) }()
url := "file:///" + file
@ -143,18 +143,68 @@ func TestAssetSerialize(t *testing.T) {
assert.Equal(t, url, archDes.URI)
assert.Equal(t, "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855", archDes.Hash)
}
{
file1, err := tempArchive("test", false)
assert.Nil(t, err)
defer func() { contract.IgnoreError(os.Remove(file1)) }()
file2, err := tempArchive("test2", false)
assert.Nil(t, err)
defer func() { contract.IgnoreError(os.Remove(file2)) }()
arch1, err := NewPathArchive(file1)
assert.Nil(t, err)
arch2, err := NewPathArchive(file2)
assert.Nil(t, err)
assert.True(t, arch1.Equals(arch2))
url := "file:///" + file1
arch3, err := NewURIArchive(url)
assert.Nil(t, err)
assert.True(t, arch1.Equals(arch3))
}
{
file, err := tempArchive("test", true)
assert.Nil(t, err)
arch1, err := NewPathArchive(file)
assert.Nil(t, err)
assert.Nil(t, arch1.EnsureHash())
url := "file:///" + file
arch2, err := NewURIArchive(url)
assert.Nil(t, err)
assert.Nil(t, arch2.EnsureHash())
assert.Nil(t, os.Truncate(file, 0))
arch3, err := NewPathArchive(file)
assert.Nil(t, err)
assert.Nil(t, arch3.EnsureHash())
assert.False(t, arch1.Equals(arch3))
arch4, err := NewURIArchive(url)
assert.Nil(t, err)
assert.Nil(t, arch4.EnsureHash())
assert.False(t, arch2.Equals(arch4))
}
}
func tempArchive(prefix string) (string, error) {
func tempArchive(prefix string, fill bool) (string, error) {
for {
path := filepath.Join(os.TempDir(), fmt.Sprintf("%s-%x.tar", prefix, rand.Uint32()))
f, err := os.OpenFile(path, os.O_RDWR|os.O_CREATE|os.O_EXCL, 0600)
if !os.IsExist(err) {
if err != nil {
defer contract.IgnoreClose(f)
// write out an empty tar file.
switch {
case os.IsExist(err):
continue
case err != nil:
return "", err
default:
defer contract.IgnoreClose(f)
// write out a tar file. if `fill` is true, add a single empty file.
if fill {
w := tar.NewWriter(f)
contract.IgnoreClose(w)
defer contract.IgnoreClose(w)
err = w.WriteHeader(&tar.Header{
Name: "file",
Mode: 0600,
Size: 0,
})
}
return path, err
}

View file

@ -109,7 +109,7 @@ func (p *provider) Diff(urn resource.URN, id resource.ID,
glog.V(7).Infof("resource[%v].Diff(id=%v,urn=%v,#olds=%v,#news=%v) executing",
p.pkg, id, urn, len(olds), len(news))
molds, err := MarshalProperties(olds, MarshalOptions{})
molds, err := MarshalProperties(olds, MarshalOptions{ElideAssetContents: true})
if err != nil {
return DiffResult{}, err
}
@ -191,7 +191,7 @@ func (p *provider) Update(urn resource.URN, id resource.ID,
glog.V(7).Infof("resource[%v].Update(id=%v,urn=%v,#olds=%v,#news=%v) executing",
p.pkg, id, urn, len(olds), len(news))
molds, err := MarshalProperties(olds, MarshalOptions{})
molds, err := MarshalProperties(olds, MarshalOptions{ElideAssetContents: true})
if err != nil {
return nil, resource.StatusOK, err
}
@ -227,7 +227,7 @@ func (p *provider) Delete(urn resource.URN, id resource.ID, props resource.Prope
contract.Assert(urn != "")
contract.Assert(id != "")
mprops, err := MarshalProperties(props, MarshalOptions{})
mprops, err := MarshalProperties(props, MarshalOptions{ElideAssetContents: true})
if err != nil {
return resource.StatusOK, err
}

View file

@ -18,6 +18,7 @@ import (
type MarshalOptions struct {
SkipNulls bool // true to skip nulls altogether in the resulting map.
KeepUnknowns bool // true if we are keeping unknown values (otherwise we skip them).
ElideAssetContents bool // true if we are eliding the contents of assets.
ComputeAssetHashes bool // true if we are computing missing asset hashes on the fly.
}
@ -336,10 +337,16 @@ func MarshalStruct(obj *structpb.Struct, opts MarshalOptions) *structpb.Value {
// MarshalAsset marshals an asset into its wire form for resource provider plugins.
func MarshalAsset(v *resource.Asset, opts MarshalOptions) (*structpb.Value, error) {
// Ensure a hash is present if needed.
if v.Hash == "" && opts.ComputeAssetHashes {
if err := v.EnsureHash(); err != nil {
return nil, errors.Wrapf(err, "failed to compute asset hash")
// If we are not providing access to an asset's contents, we simply need to record the fact that this asset existed. Serialize the
// asset with only its hash (if present).
if opts.ElideAssetContents {
v = &resource.Asset{Hash: v.Hash}
} else {
// Ensure a hash is present if needed.
if v.Hash == "" && opts.ComputeAssetHashes {
if err := v.EnsureHash(); err != nil {
return nil, errors.Wrapf(err, "failed to compute asset hash")
}
}
}
@ -351,10 +358,16 @@ func MarshalAsset(v *resource.Asset, opts MarshalOptions) (*structpb.Value, erro
// MarshalArchive marshals an archive into its wire form for resource provider plugins.
func MarshalArchive(v *resource.Archive, opts MarshalOptions) (*structpb.Value, error) {
// Ensure a hash is present if needed.
if v.Hash == "" && opts.ComputeAssetHashes {
if err := v.EnsureHash(); err != nil {
return nil, errors.Wrapf(err, "failed to compute archive hash")
// If we are not providing access to an asset's contents, we simply need to record the fact that this asset existed. Serialize the
// asset with only its hash (if present).
if opts.ElideAssetContents {
v = &resource.Archive{Hash: v.Hash}
} else {
// Ensure a hash is present if needed.
if v.Hash == "" && opts.ComputeAssetHashes {
if err := v.EnsureHash(); err != nil {
return nil, errors.Wrapf(err, "failed to compute archive hash")
}
}
}

View file

@ -298,14 +298,14 @@ func TestAssetPropertyValueDiffs(t *testing.T) {
func TestArchivePropertyValueDiffs(t *testing.T) {
t.Parallel()
path, err := tempArchive("test")
path, err := tempArchive("test", false)
assert.Nil(t, err)
defer func() { contract.IgnoreError(os.Remove(path)) }()
a1, err := NewPathArchive(path)
assert.Nil(t, err)
d1 := NewArchiveProperty(a1).Diff(NewArchiveProperty(a1))
assert.Nil(t, d1)
path2, err := tempArchive("test2")
path2, err := tempArchive("test2", true)
assert.Nil(t, err)
defer func() { contract.IgnoreError(os.Remove(path)) }()
a2, err := NewPathArchive(path2)