Skip to content
Open
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
1 change: 1 addition & 0 deletions pkg/artifact/kitfile.go
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ type (
Model struct {
Name string `json:"name,omitempty" yaml:"name,omitempty"`
Path string `json:"path,omitempty" yaml:"path,omitempty"`
BaseDigest string `json:"baseDigest,omitempty" yaml:"baseDigest,omitempty"`
License string `json:"license,omitempty" yaml:"license,omitempty"`
Framework string `json:"framework,omitempty" yaml:"framework,omitempty"`
Format string `json:"format,omitempty" yaml:"format,omitempty"`
Expand Down
61 changes: 61 additions & 0 deletions pkg/cmd/pack/pack.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,21 +18,26 @@ package pack

import (
"context"
"errors"
"fmt"
"io"
"os"
"strings"

"github.com/kitops-ml/kitops/pkg/artifact"
cmdoptions "github.com/kitops-ml/kitops/pkg/cmd/options"
"github.com/kitops-ml/kitops/pkg/lib/constants"
"github.com/kitops-ml/kitops/pkg/lib/constants/mediatype"
"github.com/kitops-ml/kitops/pkg/lib/filesystem"
"github.com/kitops-ml/kitops/pkg/lib/filesystem/ignore"
kfutils "github.com/kitops-ml/kitops/pkg/lib/kitfile"
"github.com/kitops-ml/kitops/pkg/lib/repo/local"
"github.com/kitops-ml/kitops/pkg/lib/repo/remote"
"github.com/kitops-ml/kitops/pkg/lib/repo/util"
"github.com/kitops-ml/kitops/pkg/output"

ocispec "github.com/opencontainers/image-spec/specs-go/v1"
"oras.land/oras-go/v2/errdef"
)

// runPack compresses and stores a modelkit based on a Kitfile. Returns an error if packing
Expand Down Expand Up @@ -86,6 +91,17 @@ func pack(ctx context.Context, opts *packOptions, kitfile *artifact.KitFile, loc
return nil, fmt.Errorf("Failed to resolve referenced modelkit %s: %w", kitfile.Model.Path, err)
}
extraLayerPaths = util.LayerPathsFromKitfile(parentKitfile)

baseDigest, err := resolveBaseDigest(ctx, opts.configHome, kitfile.Model.Path)
if err != nil {
output.Logf(output.LogLevelWarn, "failed to resolve base digest, keeping existing value: %v", err)
} else if baseDigest != "" {
if kitfile.Model.BaseDigest != "" && kitfile.Model.BaseDigest != baseDigest {
output.Logf(output.LogLevelWarn, "overriding user-specified baseDigest %s with resolved %s",
kitfile.Model.BaseDigest, baseDigest)
}
kitfile.Model.BaseDigest = baseDigest
Comment thread
rishi-jat marked this conversation as resolved.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

What's the point of being able to specify a base digest if it just gets overridden, pushing the digest mismatch into a single warning log line at build time.

Imagine running this build in a CI -- will you notice that the digest has changed? If not, what was the point in including it?

}
Comment thread
rishi-jat marked this conversation as resolved.
}

ignore, err := ignore.NewFromContext(opts.contextDir, kitfile, extraLayerPaths...)
Expand All @@ -112,6 +128,51 @@ func pack(ctx context.Context, opts *packOptions, kitfile *artifact.KitFile, loc
return manifestDesc, nil
}

func resolveBaseDigest(ctx context.Context, configHome, baseRef string) (string, error) {
if strings.HasPrefix(baseRef, "sha256:") {
return baseRef, nil
}

ref, _, err := artifact.ParseReference(baseRef)
if err != nil {
return "", fmt.Errorf("failed to parse base reference: %w", err)
}

localRepo, err := local.NewLocalRepo(constants.StoragePath(configHome), ref)
if err != nil {
return "", fmt.Errorf("failed to create local repository: %w", err)
}
Comment thread
rishi-jat marked this conversation as resolved.

desc, _, _, err := util.ResolveManifestAndConfig(ctx, localRepo, ref.Reference)
if err == nil {
return desc.Digest.String(), nil
}
// Expected cases: no base digest resolved locally (skip pinning, no remote fallback)
if errors.Is(err, util.ErrNoKitfile) || errors.Is(err, util.ErrNotAModelKit) {
return "", nil
}
if errors.Is(err, errdef.ErrNotFound) || errors.Is(err, errdef.ErrMissingReference) {
// Reference not found locally, try remote
} else {
// Unexpected local error (IO issues, corruption, etc.)
return "", fmt.Errorf("failed to resolve base digest locally: %w", err)
}
Comment on lines +150 to +159
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The comment here contradicts the code that follows.


repository, err := remote.NewRepository(ctx, ref.Registry, ref.Repository, cmdoptions.DefaultNetworkOptions(configHome))
Comment thread
rishi-jat marked this conversation as resolved.
if err != nil {
return "", fmt.Errorf("failed to create remote repository: %w", err)
}

desc, _, _, err = util.ResolveManifestAndConfig(ctx, repository, ref.Reference)
if err != nil {
if errors.Is(err, util.ErrNoKitfile) || errors.Is(err, util.ErrNotAModelKit) {
return "", nil
}
return "", fmt.Errorf("failed to resolve base digest remotely: %w", err)
}
Comment thread
rishi-jat marked this conversation as resolved.
return desc.Digest.String(), nil
}

func readKitfile(modelFile string) (*artifact.KitFile, error) {
// 1. Read the model file
kitfile := &artifact.KitFile{}
Expand Down
Loading