Skip to content

Commit

Permalink
Revert "Revert "Revert "Merge branch 'main' of github.com:cloudfoundr…
Browse files Browse the repository at this point in the history
…y/bosh-cli into main"""

Re-removing the change to not include recursivly dependent packages in compiled releases

We believe this is NOT actually safe, because you cannot correctly calculate the "dependency_key"
without those additional packages

This reverts commit 3041156.
  • Loading branch information
jpalermo committed Feb 2, 2022
1 parent 3041156 commit 341f966
Show file tree
Hide file tree
Showing 7 changed files with 105 additions and 8 deletions.
1 change: 1 addition & 0 deletions deployment/instance/state/remote_package_compiler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,7 @@ var _ = Describe("RemotePackageCompiler", func() {

pkg = boshpkg.NewCompiledPackageWithArchive(
"fake-package-name", "fake-package-fingerprint", "", archivePath, "fake-source-package-sha1", []string{"fake-package-name-dep"})
pkg.AttachDependencies([]*boshpkg.CompiledPackage{pkgDependency})
})

It("should skip compilation but still add blobstore package", func() {
Expand Down
13 changes: 13 additions & 0 deletions release/archive_reader.go
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,7 @@ func (r ArchiveReader) newPackages(refs []boshman.PackageRef, extractPath string

func (r ArchiveReader) newCompiledPackages(refs []boshman.CompiledPackageRef, extractPath string) ([]*boshpkg.CompiledPackage, error) {
var compiledPkgs []*boshpkg.CompiledPackage
var errs []error

for _, ref := range refs {
archivePath := filepath.Join(extractPath, "compiled_packages", ref.Name+".tgz")
Expand All @@ -196,6 +197,18 @@ func (r ArchiveReader) newCompiledPackages(refs []boshman.CompiledPackageRef, ex

compiledPkgs = append(compiledPkgs, compiledPkg)
}

for _, compiledPkg := range compiledPkgs {
err := compiledPkg.AttachDependencies(compiledPkgs)
if err != nil {
errs = append(errs, err)
}
}

if len(errs) > 0 {
return nil, bosherr.NewMultiError(errs...)
}

return compiledPkgs, nil
}

Expand Down
12 changes: 8 additions & 4 deletions release/archive_reader_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -272,6 +272,7 @@ license:
compiledPkg2 := boshpkg.NewCompiledPackageWithArchive(
"pkg2", "pkg2-fp", "pkg2-stemcell",
filepath.Join("/", "extracted", "release", "compiled_packages", "pkg2.tgz"), "pkg2-sha", nil)
compiledPkg1.AttachDependencies([]*boshpkg.CompiledPackage{compiledPkg2})

lic := boshlic.NewLicense(NewResourceWithBuiltArchive(
"license", "lic-fp", filepath.Join("/", "extracted", "release", "license.tgz"), "lic-sha"))
Expand Down Expand Up @@ -317,7 +318,7 @@ license:
Expect(job1.Packages).To(Equal([]boshpkg.Compilable{compiledPkg1}))

// compiled pkg dependencies are resolved
Expect(compiledPkg1.Dependencies).To(Equal([]*boshpkg.CompiledPackage{}))
Expect(compiledPkg1.Dependencies).To(Equal([]*boshpkg.CompiledPackage{compiledPkg2}))

Expect(fs.FileExists(filepath.Join("/", "extracted", "release"))).To(BeTrue())
})
Expand All @@ -333,7 +334,7 @@ license:
Expect(fs.FileExists("/extracted/release")).To(BeFalse())
})

It("does not return an error if compiled pkg's compiled pkg dependencies cannot be satisfied because dependencies don't matter for compiled pkgs", func() {
It("returns error if compiled pkg's compiled pkg dependencies cannot be satisfied", func() {
fs.WriteFileString(filepath.Join("/", "extracted", "release", "release.MF"), `---
name: release
version: version
Expand All @@ -348,8 +349,11 @@ compiled_packages:
`)

_, err := act()
Expect(err).ToNot(HaveOccurred())
Expect(fs.FileExists(filepath.Join("/", "extracted", "release"))).To(BeTrue())
Expect(err).To(HaveOccurred())
Expect(err.Error()).To(ContainSubstring(
"Expected to find compiled package 'pkg-with-other-name' since it's a dependency of compiled package 'pkg1'"))

Expect(fs.FileExists(filepath.Join("/", "extracted", "release"))).To(BeFalse())
})
})

Expand Down
12 changes: 12 additions & 0 deletions release/manifest_reader.go
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,7 @@ func (r ManifestReader) newPackages(refs []boshman.PackageRef) ([]*boshpkg.Packa

func (r ManifestReader) newCompiledPackages(refs []boshman.CompiledPackageRef) ([]*boshpkg.CompiledPackage, error) {
var compiledPkgs []*boshpkg.CompiledPackage
var errs []error

for _, ref := range refs {
compiledPkg := boshpkg.NewCompiledPackageWithoutArchive(
Expand All @@ -141,6 +142,17 @@ func (r ManifestReader) newCompiledPackages(refs []boshman.CompiledPackageRef) (
compiledPkgs = append(compiledPkgs, compiledPkg)
}

for _, compiledPkg := range compiledPkgs {
err := compiledPkg.AttachDependencies(compiledPkgs)
if err != nil {
errs = append(errs, err)
}
}

if len(errs) > 0 {
return nil, bosherr.NewMultiError(errs...)
}

return compiledPkgs, nil
}

Expand Down
7 changes: 5 additions & 2 deletions release/manifest_reader_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,7 @@ license:
"pkg1", "pkg1-fp", "pkg1-stemcell", "pkg1-sha", []string{"pkg2"})
compiledPkg2 := boshpkg.NewCompiledPackageWithoutArchive(
"pkg2", "pkg2-fp", "pkg2-stemcell", "pkg2-sha", nil)
compiledPkg1.AttachDependencies([]*boshpkg.CompiledPackage{compiledPkg2})

lic := boshlic.NewLicense(NewExistingResource("license", "lic-fp", "lic-sha"))

Expand All @@ -172,7 +173,7 @@ license:
Expect(release.License()).To(Equal(lic))
})

It("does not return an error if compiled pkg's compiled pkg dependencies cannot be satisfied because dependencies don't matter for compiled pkgs", func() {
It("returns error if compiled pkg's compiled pkg dependencies cannot be satisfied", func() {
fs.WriteFileString("/release.yml", `---
name: release
version: version
Expand All @@ -187,7 +188,9 @@ compiled_packages:
`)

_, err := act()
Expect(err).ToNot(HaveOccurred())
Expect(err).To(HaveOccurred())
Expect(err.Error()).To(ContainSubstring(
"Expected to find compiled package 'pkg-with-other-name' since it's a dependency of compiled package 'pkg1'"))
})
})

Expand Down
26 changes: 24 additions & 2 deletions release/pkg/compiled_package.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,15 +7,16 @@ import (

"github.com/cloudfoundry/bosh-cli/crypto"
crypto2 "github.com/cloudfoundry/bosh-utils/crypto"
bosherr "github.com/cloudfoundry/bosh-utils/errors"
)

type CompiledPackage struct {
name string
fingerprint string
osVersionSlug string

Dependencies []*CompiledPackage // todo delete; we don't care about _compiled_ package dependencies
dependencyNames []string // todo delete; we don't care about _compiled_ package dependencies
Dependencies []*CompiledPackage // todo privatize
dependencyNames []string

archivePath string
archiveDigest string
Expand Down Expand Up @@ -63,6 +64,27 @@ func (p CompiledPackage) ArchivePath() string {

func (p CompiledPackage) ArchiveDigest() string { return p.archiveDigest }

func (p *CompiledPackage) AttachDependencies(compiledPkgs []*CompiledPackage) error {
for _, pkgName := range p.dependencyNames {
var found bool

for _, compiledPkg := range compiledPkgs {
if compiledPkg.Name() == pkgName {
p.Dependencies = append(p.Dependencies, compiledPkg)
found = true
break
}
}

if !found {
errMsg := "Expected to find compiled package '%s' since it's a dependency of compiled package '%s'"
return bosherr.Errorf(errMsg, pkgName, p.name)
}
}

return nil
}

func (p *CompiledPackage) DependencyNames() []string { return p.dependencyNames }

func (p *CompiledPackage) Deps() []Compilable {
Expand Down
42 changes: 42 additions & 0 deletions release/pkg/compiled_package_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,27 @@ var _ = Describe("NewCompiledPackageWithoutArchive", func() {
Expect(compiledPkg.DependencyNames()).To(Equal([]string{"pkg1", "pkg2"}))
})
})

Describe("AttachDependencies", func() {
It("attaches dependencies based on their names", func() {
pkg1 := NewCompiledPackageWithoutArchive("pkg1", "fp", "os-slug", "sha1", nil)
pkg2 := NewCompiledPackageWithoutArchive("pkg2", "fp", "os-slug", "sha1", nil)
unusedPkg := NewCompiledPackageWithoutArchive("unused", "fp", "os-slug", "sha1", nil)

err := compiledPkg.AttachDependencies([]*CompiledPackage{pkg1, unusedPkg, pkg2})
Expect(err).ToNot(HaveOccurred())

Expect(compiledPkg.Dependencies).To(Equal([]*CompiledPackage{pkg1, pkg2}))
})

It("returns error if dependency cannot be found", func() {
pkg2 := NewCompiledPackageWithoutArchive("pkg2", "fp", "os-slug", "sha1", nil)

err := compiledPkg.AttachDependencies([]*CompiledPackage{pkg2})
Expect(err).To(HaveOccurred())
Expect(err.Error()).To(Equal("Expected to find compiled package 'pkg1' since it's a dependency of compiled package 'name'"))
})
})
})

var _ = Describe("NewCompiledPackageWithArchive", func() {
Expand Down Expand Up @@ -65,6 +86,27 @@ var _ = Describe("NewCompiledPackageWithArchive", func() {
})
})

Describe("AttachDependencies", func() {
It("attaches dependencies based on their names", func() {
pkg1 := NewCompiledPackageWithArchive("pkg1", "fp", "os-slug", "path", "sha1", nil)
pkg2 := NewCompiledPackageWithArchive("pkg2", "fp", "os-slug", "path", "sha1", nil)
unusedPkg := NewCompiledPackageWithArchive("unused", "fp", "os-slug", "path", "sha1", nil)

err := compiledPkg.AttachDependencies([]*CompiledPackage{pkg1, unusedPkg, pkg2})
Expect(err).ToNot(HaveOccurred())

Expect(compiledPkg.Dependencies).To(Equal([]*CompiledPackage{pkg1, pkg2}))
})

It("returns error if dependency cannot be found", func() {
pkg2 := NewCompiledPackageWithArchive("pkg2", "fp", "os-slug", "path", "sha1", nil)

err := compiledPkg.AttachDependencies([]*CompiledPackage{pkg2})
Expect(err).To(HaveOccurred())
Expect(err.Error()).To(Equal("Expected to find compiled package 'pkg1' since it's a dependency of compiled package 'name'"))
})
})

Describe("RehashWithCalculator", func() {
BeforeEach(func() {
fakeDigestCalculator = fakes.NewFakeDigestCalculator()
Expand Down

0 comments on commit 341f966

Please sign in to comment.