diff --git a/internal/runbits/runtime/runtime.go b/internal/runbits/runtime/runtime.go index dcdf29b446..17085edd2e 100644 --- a/internal/runbits/runtime/runtime.go +++ b/internal/runbits/runtime/runtime.go @@ -57,6 +57,8 @@ type Opts struct { ValidateBuildscript bool IgnoreAsync bool + + CheckForPrivateArtifactUpdates bool } type SetOpt func(*Opts) @@ -105,6 +107,15 @@ func WithIgnoreAsync() SetOpt { } } +// WithCheckForPrivateArtifactUpdates re-checks private artifact content even when +// the commit hash is unchanged, so re-published private artifacts are picked +// up. +func WithCheckForPrivateArtifactUpdates() SetOpt { + return func(opts *Opts) { + opts.CheckForPrivateArtifactUpdates = true + } +} + type primeable interface { primer.Projecter primer.Auther @@ -186,7 +197,7 @@ func Update( prime.Output().Notice(output.Title(locale.T("install_runtime"))) } - if rt.Hash() == rtHash { + if rt.Hash() == rtHash && (!opts.CheckForPrivateArtifactUpdates || !rt.HasPrivateArtifacts()) { prime.Output().Notice(locale.T("pkg_already_uptodate")) return rt, nil } @@ -288,8 +299,11 @@ func Update( rtOpts = append(rtOpts, runtime.WithPortable()) } rtOpts = append(rtOpts, runtime.WithCacheSize(prime.Config().GetInt(constants.RuntimeCacheSizeConfigKey))) + if opts.CheckForPrivateArtifactUpdates { + rtOpts = append(rtOpts, runtime.WithCheckForPrivateArtifactUpdates()) + } - // Fetch the organization key for private ingredients, if a key service is configured. + // Fetch the organization key for private artifacts, if a key service is configured. orgKeyProvider := orgkey.New(prime.Config(), proj.Owner()) if orgKeyProvider.Configured() { defer orgKeyProvider.Close() diff --git a/internal/runners/pull/pull.go b/internal/runners/pull/pull.go index 4048e4097d..df7dfd3ef0 100644 --- a/internal/runners/pull/pull.go +++ b/internal/runners/pull/pull.go @@ -218,7 +218,7 @@ func (p *Pull) Run(params *PullParams) (rerr error) { }) } - _, err = runtime_runbit.Update(p.prime, trigger.TriggerPull) + _, err = runtime_runbit.Update(p.prime, trigger.TriggerPull, runtime_runbit.WithCheckForPrivateArtifactUpdates()) if err != nil { return locale.WrapError(err, "err_pull_refresh", "Could not refresh runtime after pull") } diff --git a/internal/runners/refresh/refresh.go b/internal/runners/refresh/refresh.go index 41a3e3c352..4f45a0f0e2 100644 --- a/internal/runners/refresh/refresh.go +++ b/internal/runners/refresh/refresh.go @@ -92,11 +92,19 @@ func (r *Refresh) Run(params *Params) error { } if !needsUpdate { - r.out.Notice(locale.T("refresh_runtime_uptodate")) - return nil + // Even when the commit hash is unchanged, a private artifact may have been + // re-published under the same artifact ID; those runtimes still need a refresh. + hasPrivate, err := runtime_helpers.HasPrivateArtifacts(proj) + if err != nil { + return errs.Wrap(err, "could not determine if runtime has private artifacts") + } + if !hasPrivate { + r.out.Notice(locale.T("refresh_runtime_uptodate")) + return nil + } } - rti, err := runtime_runbit.Update(r.prime, trigger.TriggerRefresh, runtime_runbit.WithoutHeaders(), runtime_runbit.WithIgnoreAsync()) + rti, err := runtime_runbit.Update(r.prime, trigger.TriggerRefresh, runtime_runbit.WithoutHeaders(), runtime_runbit.WithIgnoreAsync(), runtime_runbit.WithCheckForPrivateArtifactUpdates()) if err != nil { return locale.WrapError(err, "err_refresh_runtime_new", "Could not update runtime for this project.") } diff --git a/pkg/runtime/decrypt_test.go b/pkg/runtime/decrypt_test.go index 075726f1c7..ab592a52cf 100644 --- a/pkg/runtime/decrypt_test.go +++ b/pkg/runtime/decrypt_test.go @@ -226,6 +226,112 @@ func TestPrivateArtifactSurvivesEviction(t *testing.T) { } } +func TestMarkPrivateStoresChecksum(t *testing.T) { + id := strfmt.UUID("11111111-1111-1111-1111-111111111111") + d := &depot{ + config: depotConfig{ + Deployments: map[strfmt.UUID][]deployment{}, + Cache: map[strfmt.UUID]*artifactInfo{id: {Size: 1}}, // pre-seeded so no on-disk size lookup + }, + depotPath: t.TempDir(), + artifacts: map[strfmt.UUID]struct{}{}, + } + + if err := d.MarkPrivate(id, "sha256:abc"); err != nil { + t.Fatalf("MarkPrivate: %v", err) + } + info := d.config.Cache[id] + if !info.Private { + t.Error("artifact was not marked private") + } + if info.Checksum != "sha256:abc" { + t.Errorf("stored checksum = %q, want sha256:abc", info.Checksum) + } +} + +func TestPrivateContentChanged(t *testing.T) { + const ( + privFresh = strfmt.UUID("aaaaaaaa-0000-0000-0000-000000000000") + privStale = strfmt.UUID("bbbbbbbb-0000-0000-0000-000000000000") + public = strfmt.UUID("cccccccc-0000-0000-0000-000000000000") + absent = strfmt.UUID("dddddddd-0000-0000-0000-000000000000") + ) + d := &depot{ + config: depotConfig{ + Cache: map[strfmt.UUID]*artifactInfo{ + privFresh: {Private: true, Checksum: "sha256:aaa"}, + privStale: {Private: true, Checksum: "sha256:old"}, + public: {Checksum: "sha256:pub"}, // not private + }, + }, + artifacts: map[strfmt.UUID]struct{}{}, + } + + cases := []struct { + name string + id strfmt.UUID + checksum string + want bool + }{ + {"fresh private matches", privFresh, "sha256:aaa", false}, + {"stale private mismatches", privStale, "sha256:new", true}, + {"public artifact is never stale", public, "sha256:different", false}, + {"absent entry", absent, "sha256:any", false}, + {"empty build-plan checksum does not churn", privStale, "", false}, + } + for _, tc := range cases { + if got := d.PrivateContentChanged(tc.id, tc.checksum); got != tc.want { + t.Errorf("%s: PrivateContentChanged = %v, want %v", tc.name, got, tc.want) + } + } +} + +func TestHasPrivateArtifacts(t *testing.T) { + const ( + priv = strfmt.UUID("aaaaaaaa-0000-0000-0000-000000000000") + public = strfmt.UUID("bbbbbbbb-0000-0000-0000-000000000000") + ) + path := t.TempDir() + other := t.TempDir() + + newDepotWith := func(cache map[strfmt.UUID]*artifactInfo, deps map[strfmt.UUID][]deployment) *depot { + return &depot{ + config: depotConfig{Cache: cache, Deployments: deps}, + artifacts: map[strfmt.UUID]struct{}{}, + } + } + + t.Run("private deployed at path", func(t *testing.T) { + d := newDepotWith( + map[strfmt.UUID]*artifactInfo{priv: {Private: true}}, + map[strfmt.UUID][]deployment{priv: {{Path: path}}}, + ) + if !d.HasPrivateArtifacts(path) { + t.Error("expected the private deployment to be detected") + } + }) + + t.Run("only public deployed", func(t *testing.T) { + d := newDepotWith( + map[strfmt.UUID]*artifactInfo{public: {}}, + map[strfmt.UUID][]deployment{public: {{Path: path}}}, + ) + if d.HasPrivateArtifacts(path) { + t.Error("a public-only runtime should not report private deployments") + } + }) + + t.Run("private deployed only at another path", func(t *testing.T) { + d := newDepotWith( + map[strfmt.UUID]*artifactInfo{priv: {Private: true}}, + map[strfmt.UUID][]deployment{priv: {{Path: other}}}, + ) + if d.HasPrivateArtifacts(path) { + t.Error("a private deployment at another path should not match") + } + }) +} + func exists(path string) bool { _, err := os.Stat(path) return err == nil diff --git a/pkg/runtime/depot.go b/pkg/runtime/depot.go index a558400442..d44910a3cd 100644 --- a/pkg/runtime/depot.go +++ b/pkg/runtime/depot.go @@ -60,6 +60,9 @@ type artifactInfo struct { // For private decrypted artifacts. Private bool `json:"private,omitempty"` + // Checksum is the build-plan content checksum a private artifact was stored + // under, used to detect re-published content for a fixed artifact ID. + Checksum string `json:"checksum,omitempty"` id strfmt.UUID // for convenience when removing stale artifacts; should NOT have json tag } @@ -195,9 +198,11 @@ func (d *depot) Put(id strfmt.UUID) error { return nil } -// MarkPrivate flags an artifact as a decrypted private artifact, ensuring a -// cache entry exists for it. Private artifacts are exempt from stale removal. -func (d *depot) MarkPrivate(id strfmt.UUID) error { +// MarkPrivate flags an artifact as a decrypted private artifact and records the +// build-plan checksum its content was stored under, ensuring a cache entry +// exists for it. Private artifacts are exempt from stale removal, and the stored +// checksum lets the next update detect re-published content. +func (d *depot) MarkPrivate(id strfmt.UUID, checksum string) error { d.mapMutex.Lock() defer d.mapMutex.Unlock() @@ -209,9 +214,47 @@ func (d *depot) MarkPrivate(id strfmt.UUID) error { d.config.Cache[id] = &artifactInfo{Size: size, id: id} } d.config.Cache[id].Private = true + d.config.Cache[id].Checksum = checksum return nil } +// HasPrivateArtifacts reports whether any artifact deployed at path is a private +// artifact. A runtime with private artifacts cannot be trusted as up-to-date +// by commit hash alone, since the same artifact ID may point at re-published +// content. +func (d *depot) HasPrivateArtifacts(path string) bool { + d.mapMutex.Lock() + defer d.mapMutex.Unlock() + + resolved := fileutils.ResolvePathIfPossible(path) + for id, deploys := range d.config.Deployments { + info, ok := d.config.Cache[id] + if !ok || !info.Private { + continue + } + for _, dep := range deploys { + if fileutils.ResolvePathIfPossible(dep.Path) == resolved { + return true + } + } + } + return false +} + +// PrivateContentChanged reports whether the depot holds a private artifact for id +// whose stored content checksum differs from the given build-plan checksum — the +// timeless/re-published case where a fixed artifact ID now points at new content. +func (d *depot) PrivateContentChanged(id strfmt.UUID, checksum string) bool { + d.mapMutex.Lock() + defer d.mapMutex.Unlock() + + info, exists := d.config.Cache[id] + if !exists || !info.Private || checksum == "" { + return false + } + return info.Checksum != checksum +} + // DeployViaLink will take an artifact from the depot and link it to the target path. // It should return deployment info to be used for tracking the artifact. func (d *depot) DeployViaLink(id strfmt.UUID, relativeSrc, absoluteDest string) (*deployment, error) { diff --git a/pkg/runtime/options.go b/pkg/runtime/options.go index 63431b74a7..53151bdf58 100644 --- a/pkg/runtime/options.go +++ b/pkg/runtime/options.go @@ -24,6 +24,12 @@ func WithDecryptionKey(key []byte, keyID string) SetOpt { } } +// WithCheckForPrivateArtifactUpdates makes the update re-check private artifact +// content even when the commit hash is unchanged (re-published content). +func WithCheckForPrivateArtifactUpdates() SetOpt { + return func(opts *Opts) { opts.CheckForPrivateArtifactUpdates = true } +} + func WithBuildlogFilePath(path string) SetOpt { return func(opts *Opts) { opts.BuildlogFilePath = path } } diff --git a/pkg/runtime/runtime.go b/pkg/runtime/runtime.go index 5f4b255e31..3258251701 100644 --- a/pkg/runtime/runtime.go +++ b/pkg/runtime/runtime.go @@ -77,17 +77,23 @@ func (r *Runtime) HasCache() bool { return r.hash != "" } -func (r *Runtime) Update(bp *buildplan.BuildPlan, hash string, setOpts ...SetOpt) error { - if r.hash == hash { - logging.Debug("Runtime is already up to date") - return nil - } +// HasPrivateArtifacts reports whether this runtime has any private (decrypted) +// artifacts deployed, whose content may change under an unchanged commit hash. +func (r *Runtime) HasPrivateArtifacts() bool { + return r.depot.HasPrivateArtifacts(r.path) +} +func (r *Runtime) Update(bp *buildplan.BuildPlan, hash string, setOpts ...SetOpt) error { opts := &Opts{} for _, setOpt := range setOpts { setOpt(opts) } + if r.hash == hash && (!opts.CheckForPrivateArtifactUpdates || !r.HasPrivateArtifacts()) { + logging.Debug("Runtime is already up to date") + return nil + } + if opts.BuildlogFilePath == "" { opts.BuildlogFilePath = filepath.Join(r.path, configDir, buildLogFile) } diff --git a/pkg/runtime/setup.go b/pkg/runtime/setup.go index 84023f47f3..889148ff96 100644 --- a/pkg/runtime/setup.go +++ b/pkg/runtime/setup.go @@ -61,10 +61,15 @@ type Opts struct { // OrgKey is the organization AES-256 key used to decrypt private artifacts // during install, with OrgKeyID identifying which key it is. Both are empty - // when the runtime has no private ingredients. + // when the runtime has no private artifacts. OrgKey []byte OrgKeyID string + // CheckForPrivateArtifactUpdates makes the update re-solve and re-check + // content even when the commit hash is unchanged, so a private artifact + // re-published under the same artifact ID is detected. + CheckForPrivateArtifactUpdates bool + FromArchive *fromArchive // Annotations are used strictly to pass information for the purposes of analytics @@ -139,28 +144,44 @@ func newSetup(path string, bp *buildplan.BuildPlan, env *envdef.Collection, depo // Start off with the full range of artifacts relevant to our platform installableArtifacts := bp.Artifacts(filterInstallable...) + installableArtifactsMap := installableArtifacts.ToIDMap() + + // Identify installed private artifacts whose depot content no longer matches + // the build plan: a private artifact re-published under the same artifact ID. + statePrivateArtifacts := map[strfmt.UUID]bool{} + for id := range installedArtifacts { + a, required := installableArtifactsMap[id] + if required && depot.PrivateContentChanged(id, a.Checksum) { + logging.Debug("Private artifact %s content changed; re-fetching", id) + statePrivateArtifacts[id] = true + } + } - // Identify which artifacts we'll need to install, this filters out any artifacts that are already installed. + // Identify which artifacts we'll need to install. This filters out artifacts + // that are already installed, except stale private artifacts which must be + // re-installed with their new content. artifactsToInstall := installableArtifacts.Filter(func(a *buildplan.Artifact) bool { _, installed := installedArtifacts[a.ArtifactID] - return !installed + return !installed || statePrivateArtifacts[a.ArtifactID] }) // Identify which artifacts we can uninstall - installableArtifactsMap := installableArtifacts.ToIDMap() artifactsToUninstall := map[strfmt.UUID]bool{} for id := range installedArtifacts { if _, required := installableArtifactsMap[id]; !required { artifactsToUninstall[id] = true } } + for id := range statePrivateArtifacts { + artifactsToUninstall[id] = true + } // Calculate which artifacts need to be downloaded; if an artifact we want to install is not in our depot then // by definition we'll need to download it (unless we're setting up the runtime from an archive). // We also calculate which artifacts are immediately ready to be installed, as its the inverse condition of the above. artifactsToDownload := artifactsToInstall.Filter(func(a *buildplan.Artifact) bool { exists, _ := depot.Exists(a.ArtifactID) - return !exists + return !exists || statePrivateArtifacts[a.ArtifactID] }) artifactsToUnpack := artifactsToDownload if opts.FromArchive != nil { @@ -476,6 +497,12 @@ func (s *setup) unpack(artifact *buildplan.Artifact, b []byte) (rerr error) { }, }, bytes.NewReader(b)) unpackPath := s.depot.Path(artifact.ArtifactID) + if fileutils.DirExists(unpackPath) { + // Clear prior private artifact that is being updated. + if err := os.RemoveAll(unpackPath); err != nil { + return errs.Wrap(err, "could not clear private artifact directory") + } + } if err := ua.Unarchive(proxy, unpackPath); err != nil { if err2 := os.RemoveAll(unpackPath); err2 != nil { return errs.Pack(err, errs.Wrap(err2, "unable to remove partially-unpacked directory")) @@ -505,7 +532,7 @@ func (s *setup) unpack(artifact *buildplan.Artifact, b []byte) (rerr error) { } if outcome == decryptDone { logging.Debug("Decrypted private artifact %s (%s)", artifact.ArtifactID, artifact.Name()) - if err := s.depot.MarkPrivate(artifact.ArtifactID); err != nil { + if err := s.depot.MarkPrivate(artifact.ArtifactID, artifact.Checksum); err != nil { return errs.Wrap(err, "Could not mark decrypted artifact as private") } } diff --git a/pkg/runtime_helpers/helpers.go b/pkg/runtime_helpers/helpers.go index 3ecad8c694..79ba067b31 100644 --- a/pkg/runtime_helpers/helpers.go +++ b/pkg/runtime_helpers/helpers.go @@ -45,6 +45,14 @@ func NeedsUpdate(proj *project.Project, overrideCommitID *strfmt.UUID) (bool, er return hash != rt.Hash(), nil } +func HasPrivateArtifacts(proj *project.Project) (bool, error) { + rt, err := FromProject(proj) + if err != nil { + return false, errs.Wrap(err, "Could not obtain runtime") + } + return rt.HasPrivateArtifacts(), nil +} + func Hash(proj *project.Project, overrideCommitID *strfmt.UUID) (string, error) { var err error var commitID strfmt.UUID