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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Resolve bundle digests when DepV2 is enabled #3071

Merged
merged 4 commits into from
May 16, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
72 changes: 71 additions & 1 deletion pkg/porter/generateManifest.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,15 @@ import (
"strings"

cnabtooci "get.porter.sh/porter/pkg/cnab/cnab-to-oci"
"get.porter.sh/porter/pkg/experimental"

"get.porter.sh/porter/pkg"
"get.porter.sh/porter/pkg/build"
"get.porter.sh/porter/pkg/cnab"
"get.porter.sh/porter/pkg/manifest"
"get.porter.sh/porter/pkg/tracing"
"get.porter.sh/porter/pkg/yaml"
"github.com/docker/distribution/reference"
"github.com/mikefarah/yq/v3/pkg/yqlib"
"github.com/opencontainers/go-digest"
"go.opentelemetry.io/otel/attribute"
Expand Down Expand Up @@ -111,15 +113,83 @@ func (p *Porter) generateInternalManifest(ctx context.Context, opts BuildOptions
}
}
return e.SetValue(path+"digest", digest.String())

})
if err != nil {
return err
}

if p.IsFeatureEnabled(experimental.FlagDependenciesV2) {
Copy link
Member

Choose a reason for hiding this comment

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

Same as below (?) - I would love to see this moved into here - we can use this too :)

if err = p.resolveDependencyDigest(ctx, e, regOpts); err != nil {
return err
}
}

return e.WriteFile(build.LOCAL_MANIFEST)
}

func (p *Porter) resolveDependencyDigest(ctx context.Context, e *yaml.Editor, opts cnabtooci.RegistryOptions) error {
Copy link
Member

Choose a reason for hiding this comment

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

Could this be reworked to leverage existing dependency resolution logic? - moving this into "validate" logic may help us reduce LOC and improve efficiency :)

// find all referenced dependencies that does not have digest specified
// get the digest for all of them and update the manifest with the digest
return e.WalkNodes(ctx, "dependencies.requires.*", func(ctx context.Context, nc *yqlib.NodeContext) error {
Copy link
Member

Choose a reason for hiding this comment

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

My only concern is

  1. Should we resolve dependencies that aren't required (I feel yes?) - and:
  2. Are there any concerns with this and "shared dependencies", where the dependency is already installed and running (I think this should be okay)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

  1. I'm not sure I fully understand what you mean with "dependencies not required"
  2. I think it should be okay too, but I will create a test to verify it. It makes sense long term anyway

Copy link
Contributor Author

@kichristensen kichristensen Apr 11, 2024

Choose a reason for hiding this comment

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

@schristoff Number 2 Should be covered by the existing integration for shared dependencies,

func TestSharedDependencies(t *testing.T) {

Do you agree?

ctx, span := tracing.StartSpanWithName(ctx, "updateDependencyTagToDigest")
defer span.EndSpan()

dep := &manifest.Dependency{}
if err := nc.Node.Decode(dep); err != nil {
return span.Errorf("failed to deserialize dependency in manifest: %w", err)
}

span.SetAttributes(attribute.String("dependency", dep.Name))

bundleOpts := BundleReferenceOptions{
BundlePullOptions: BundlePullOptions{
Reference: dep.Bundle.Reference,
InsecureRegistry: opts.InsecureRegistry,
},
}

ref, err := cnab.ParseOCIReference(dep.Bundle.Reference)
if err != nil {
return span.Errorf("failed to parse OCI reference for dependency %s: %w", dep.Name, err)
}

if ref.Tag() == "" || ref.Tag() == "latest" {
return nil
}

bundleRef, err := p.resolveBundleReference(ctx, &bundleOpts)
if err != nil {
return span.Errorf("failed to resolve dependency %s: %w", dep.Name, err)
}

digest := bundleRef.Digest
span.SetAttributes(attribute.String("digest", digest.Encoded()))

var path string
for _, p := range nc.PathStack {
switch t := p.(type) {
case string:
path += fmt.Sprintf("%s.", t)
case int:
path = strings.TrimSuffix(path, ".")
path += fmt.Sprintf("[%s].", strconv.Itoa(t))
default:
continue
}
}

newRef := cnab.OCIReference{
Named: reference.TrimNamed(bundleRef.Reference.Named),
}
refWithDigest, err := newRef.WithDigest(digest)
if err != nil {
return span.Errorf("failed to set digest: %w", err)
}

return e.SetValue(path+"bundle.reference", refWithDigest.String())
})
}

// getImageDigest retrieves the repository digest associated with the specified image reference.
func (p *Porter) getImageDigest(ctx context.Context, img cnab.OCIReference, regOpts cnabtooci.RegistryOptions) (digest.Digest, error) {
ctx, span := tracing.StartSpan(ctx, attribute.String("image", img.String()))
Expand Down
79 changes: 79 additions & 0 deletions pkg/porter/generateManifest_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,11 @@ import (
"testing"

"get.porter.sh/porter/pkg/build"
"get.porter.sh/porter/pkg/cache"
"get.porter.sh/porter/pkg/cnab"
cnabtooci "get.porter.sh/porter/pkg/cnab/cnab-to-oci"
"get.porter.sh/porter/pkg/config"
"get.porter.sh/porter/pkg/experimental"
"get.porter.sh/porter/pkg/test"
"github.com/docker/docker/api/types"
"github.com/stretchr/testify/require"
Expand Down Expand Up @@ -184,3 +186,80 @@ func Test_getImageLatestDigest(t *testing.T) {
})
}
}

func Test_depv2_bundleDigest(t *testing.T) {
defaultMockFindBundle := func(ref cnab.OCIReference) (cache.CachedBundle, bool, error) {
cachedBundle := cache.CachedBundle{
BundleReference: cnab.BundleReference{
Reference: ref,
Digest: "sha256:3abc67269f59e3ed824e811a1ff1ee64f0d44c0218efefada57a4bebc2d7ef6f",
},
}

return cachedBundle, true, nil
}

testcases := []struct {
name string
originalManifest string
wantManifest string
wantErr string
mockFindBundle func(ref cnab.OCIReference) (cache.CachedBundle, bool, error)
mockPullBundle func(ctx context.Context, ref cnab.OCIReference, opts cnabtooci.RegistryOptions) (cnab.BundleReference, error)
}{
{
name: "use digest in bundle reference",
wantManifest: "expected-result-depv2.yaml",
originalManifest: "original-depv2.yaml",
},
{
name: "not found reference",
wantManifest: "expected-result-depv2.yaml",
originalManifest: "original-depv2.yaml",
mockFindBundle: func(ref cnab.OCIReference) (cache.CachedBundle, bool, error) {
return cache.CachedBundle{}, false, nil
},
mockPullBundle: func(ctx context.Context, ref cnab.OCIReference, opts cnabtooci.RegistryOptions) (cnab.BundleReference, error) {
return cnab.BundleReference{}, errors.New("failed to pull bundle")
},
wantErr: "failed to pull bundle",
},
{
name: "no default bundle reference",
wantManifest: "expected-result-depv2-no-default-ref.yaml",
originalManifest: "original-depv2-no-default-ref.yaml",
},
}

for _, tc := range testcases {
t.Run(tc.name, func(t *testing.T) {
p := NewTestPorter(t)
p.SetExperimentalFlags(experimental.FlagDependenciesV2)
defer p.Close()
if tc.mockFindBundle != nil {
p.TestCache.FindBundleMock = tc.mockFindBundle
} else {
p.TestCache.FindBundleMock = defaultMockFindBundle
}
if tc.mockPullBundle != nil {
p.TestRegistry.MockPullBundle = tc.mockPullBundle
}
p.TestConfig.TestContext.AddTestFile(filepath.Join("testdata/generateManifest", tc.originalManifest), config.Name)
opts := BuildOptions{}
opts.Validate(p.Porter)

err := p.generateInternalManifest(context.Background(), opts)
if tc.wantErr != "" {
require.ErrorContains(t, err, tc.wantErr)
return
}
require.NoError(t, err)

goldenFile := filepath.Join("testdata/generateManifest", tc.wantManifest)
p.TestConfig.TestContext.AddTestFile(goldenFile, tc.wantManifest)
got, err := p.FileSystem.ReadFile(build.LOCAL_MANIFEST)
require.NoError(t, err)
test.CompareGoldenFile(t, goldenFile, string(got))
})
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
schemaVersion: 1.0.0-alpha.1
name: porter-hello
version: 0.1.0
description: "An example Porter configuration"
registry: "localhost:5000"
mixins:
- exec
dependencies:
requires:
- name: mysql
bundle:
reference: getporter/mysql
- name: mysqlLatest
bundle:
reference: getporter/mysql:latest
install:
- exec:
description: "Install Hello World"
command: ./helpers.sh
arguments:
- install
status:
- exec:
description: "World Status"
command: ./helpers.sh
arguments:
- status
uninstall:
- exec:
description: "Uninstall Hello World"
command: ./helpers.sh
arguments:
- uninstall
30 changes: 30 additions & 0 deletions pkg/porter/testdata/generateManifest/expected-result-depv2.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
schemaVersion: 1.0.0-alpha.1
name: porter-hello
version: 0.1.0
description: "An example Porter configuration"
registry: "localhost:5000"
mixins:
- exec
dependencies:
requires:
- name: mysql
bundle:
reference: getporter/mysql@sha256:3abc67269f59e3ed824e811a1ff1ee64f0d44c0218efefada57a4bebc2d7ef6f
install:
- exec:
description: "Install Hello World"
command: ./helpers.sh
arguments:
- install
status:
- exec:
description: "World Status"
command: ./helpers.sh
arguments:
- status
uninstall:
- exec:
description: "Uninstall Hello World"
command: ./helpers.sh
arguments:
- uninstall
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
schemaVersion: 1.0.0-alpha.1
name: porter-hello
version: 0.1.0
description: "An example Porter configuration"
registry: "localhost:5000"
mixins:
- exec
dependencies:
requires:
- name: mysql
bundle:
reference: getporter/mysql
- name: mysqlLatest
bundle:
reference: getporter/mysql:latest
install:
- exec:
description: "Install Hello World"
command: ./helpers.sh
arguments:
- install
status:
- exec:
description: "World Status"
command: ./helpers.sh
arguments:
- status
uninstall:
- exec:
description: "Uninstall Hello World"
command: ./helpers.sh
arguments:
- uninstall
30 changes: 30 additions & 0 deletions pkg/porter/testdata/generateManifest/original-depv2.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
schemaVersion: 1.0.0-alpha.1
name: porter-hello
version: 0.1.0
description: "An example Porter configuration"
registry: "localhost:5000"
mixins:
- exec
dependencies:
requires:
- name: mysql
bundle:
reference: getporter/mysql:v0.1.4
install:
- exec:
description: "Install Hello World"
command: ./helpers.sh
arguments:
- install
status:
- exec:
description: "World Status"
command: ./helpers.sh
arguments:
- status
uninstall:
- exec:
description: "Uninstall Hello World"
command: ./helpers.sh
arguments:
- uninstall
Loading