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

fix: OCI content created by "host" builder on Windows #1871

Merged
merged 1 commit into from
Jul 18, 2023
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
89 changes: 85 additions & 4 deletions pkg/oci/builder_test.go
Original file line number Diff line number Diff line change
@@ -1,15 +1,22 @@
package oci

import (
"archive/tar"
"compress/gzip"
"context"
"encoding/json"
"errors"
"fmt"
"io"
"io/fs"
"os"
"path/filepath"
"runtime"
"sort"
"strings"
"testing"

"github.com/google/go-cmp/cmp"
v1 "github.com/google/go-containerregistry/pkg/v1"
fn "knative.dev/func/pkg/functions"
. "knative.dev/func/pkg/testing"
Expand Down Expand Up @@ -172,8 +179,82 @@ func validateOCI(path string, t *testing.T) {
t.Fatalf("invalid schema version, expected 2, got %d", imageIndex.SchemaVersion)
}

// Additional validation of the Image Index structure can be added here
// extract. for example checking that the path includes the README.md
// and one of the binaries in the exact location expected (the data layer
// blob and exec layer blob, respectively)
if len(imageIndex.Manifests) < 1 {
t.Fatal("fewer manifests")
}

digest := strings.TrimPrefix(imageIndex.Manifests[0].Digest, "sha256:")
manifestFile := filepath.Join(path, "blobs", "sha256", digest)
manifestFileData, err := os.ReadFile(manifestFile)
if err != nil {
t.Fatal(err)
}
mf := struct {
Layers []struct {
Digest string `json:"digest"`
} `json:"layers"`
}{}
err = json.Unmarshal(manifestFileData, &mf)
if err != nil {
t.Fatal(err)
}

type fileInfo struct {
Path string
Type fs.FileMode
Executable bool
}
var files []fileInfo

for _, layer := range mf.Layers {
func() {
digest = strings.TrimPrefix(layer.Digest, "sha256:")
f, err := os.Open(filepath.Join(path, "blobs", "sha256", digest))
if err != nil {
t.Fatal(err)
}
defer f.Close()

gr, err := gzip.NewReader(f)
if err != nil {
t.Fatal(err)
}
defer gr.Close()

tr := tar.NewReader(gr)
for {
hdr, err := tr.Next()
if err != nil {
if errors.Is(err, io.EOF) {
break
}
t.Fatal(err)
}
files = append(files, fileInfo{
Path: hdr.Name,
Type: hdr.FileInfo().Mode() & fs.ModeType,
Executable: (hdr.FileInfo().Mode()&0111 == 0111) && !hdr.FileInfo().IsDir(),
})
}
}()
}
sort.Slice(files, func(i, j int) bool {
return files[i].Path < files[j].Path
})

expectedFiles := []fileInfo{
{Path: "/etc/pki/tls/certs/ca-certificates.crt"},
{Path: "/etc/ssl/certs/ca-certificates.crt"},
{Path: "/func", Type: fs.ModeDir},
{Path: "/func/README.md"},
{Path: "/func/f", Executable: true},
{Path: "/func/func.yaml"},
{Path: "/func/go.mod"},
{Path: "/func/handle.go"},
{Path: "/func/handle_test.go"},
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@lkingland why are the sources in the image? Shouldn't be there only executable and certificates?

Copy link
Member

Choose a reason for hiding this comment

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

The third layer is the data layer (for example, a repository which is a static HTTP site and a .go function which serves the files on request). The goal here was to use a .funcignore pre-populated with *.go when that feature is available. I will open a separate issue for that

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@lkingland I am afraid that if you add *.gos to .funcignore then the source will be also ignored by builders -- it won't compile application at all.

Copy link
Member

Choose a reason for hiding this comment

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

It could, depending on how we interpret the .funcignore per builder

I opened a discussion


if diff := cmp.Diff(expectedFiles, files); diff != "" {
t.Error("files in oci differ from expectation (-want, +got):", diff)
}
}
3 changes: 2 additions & 1 deletion pkg/oci/containerize.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"fmt"
"io"
"os"
slashpath "path"
"path/filepath"
"strings"

Expand Down Expand Up @@ -159,7 +160,7 @@ func newDataTarball(source, target string, ignored []string, verbose bool) error
return err
}

header.Name = filepath.Join("/func", relPath)
header.Name = slashpath.Join("/func", filepath.ToSlash(relPath))
// TODO: should we set file timestamps to the build start time of cfg.t?
// header.ModTime = timestampArgument

Expand Down
5 changes: 4 additions & 1 deletion pkg/oci/containerize_go.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,10 @@ import (
"compress/gzip"
"fmt"
"io"
"io/fs"
"os"
"os/exec"
slashpath "path"
"path/filepath"
"strings"

Expand Down Expand Up @@ -169,8 +171,9 @@ func newExecTarball(source, target string, verbose bool) error {
if err != nil {
return err
}
header.Mode = (header.Mode & ^int64(fs.ModePerm)) | 0755

header.Name = path("/func", "f")
header.Name = slashpath.Join("/func", "f")
// TODO: should we set file timestamps to the build start time of cfg.t?
// header.ModTime = timestampArgument

Expand Down
Loading