-
Notifications
You must be signed in to change notification settings - Fork 0
Fix oci image #1
Changes from all commits
54d6d32
b85c8c3
ea9d2ba
dbfc338
5869ab8
1b71c7b
2c36dbb
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,12 +1,62 @@ | ||
module roob.re/reroller | ||
module github.com/newrelic-forks/reroller | ||
|
||
go 1.16 | ||
go 1.20 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We are having issues with Go 1.20 and Alpine. We think that it is because we are compiling with |
||
|
||
require ( | ||
github.com/sirupsen/logrus v1.8.1 | ||
github.com/sirupsen/logrus v1.9.0 | ||
github.com/spf13/pflag v1.0.5 | ||
github.com/spf13/viper v1.12.0 | ||
k8s.io/api v0.24.1 | ||
k8s.io/apimachinery v0.24.1 | ||
k8s.io/client-go v0.24.1 | ||
github.com/spf13/viper v1.15.0 | ||
github.com/stretchr/testify v1.8.2 | ||
k8s.io/api v0.26.3 | ||
k8s.io/apimachinery v0.26.3 | ||
k8s.io/client-go v0.26.3 | ||
) | ||
|
||
require ( | ||
github.com/davecgh/go-spew v1.1.1 // indirect | ||
github.com/emicklei/go-restful/v3 v3.9.0 // indirect | ||
github.com/fsnotify/fsnotify v1.6.0 // indirect | ||
github.com/go-logr/logr v1.2.3 // indirect | ||
github.com/go-openapi/jsonpointer v0.19.5 // indirect | ||
github.com/go-openapi/jsonreference v0.20.0 // indirect | ||
github.com/go-openapi/swag v0.19.14 // indirect | ||
github.com/gogo/protobuf v1.3.2 // indirect | ||
github.com/golang/protobuf v1.5.2 // indirect | ||
github.com/google/gnostic v0.5.7-v3refs // indirect | ||
github.com/google/go-cmp v0.5.9 // indirect | ||
github.com/google/gofuzz v1.1.0 // indirect | ||
github.com/hashicorp/hcl v1.0.0 // indirect | ||
github.com/imdario/mergo v0.3.6 // indirect | ||
github.com/josharian/intern v1.0.0 // indirect | ||
github.com/json-iterator/go v1.1.12 // indirect | ||
github.com/magiconair/properties v1.8.7 // indirect | ||
github.com/mailru/easyjson v0.7.6 // indirect | ||
github.com/mitchellh/mapstructure v1.5.0 // indirect | ||
github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd // indirect | ||
github.com/modern-go/reflect2 v1.0.2 // indirect | ||
github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822 // indirect | ||
github.com/pelletier/go-toml/v2 v2.0.6 // indirect | ||
github.com/pmezard/go-difflib v1.0.0 // indirect | ||
github.com/spf13/afero v1.9.3 // indirect | ||
github.com/spf13/cast v1.5.0 // indirect | ||
github.com/spf13/jwalterweatherman v1.1.0 // indirect | ||
github.com/subosito/gotenv v1.4.2 // indirect | ||
golang.org/x/net v0.7.0 // indirect | ||
golang.org/x/oauth2 v0.0.0-20221014153046-6fdb5e3db783 // indirect | ||
golang.org/x/sys v0.5.0 // indirect | ||
golang.org/x/term v0.5.0 // indirect | ||
golang.org/x/text v0.7.0 // indirect | ||
golang.org/x/time v0.1.0 // indirect | ||
google.golang.org/appengine v1.6.7 // indirect | ||
google.golang.org/protobuf v1.28.1 // indirect | ||
gopkg.in/inf.v0 v0.9.1 // indirect | ||
gopkg.in/ini.v1 v1.67.0 // indirect | ||
gopkg.in/yaml.v2 v2.4.0 // indirect | ||
gopkg.in/yaml.v3 v3.0.1 // indirect | ||
k8s.io/klog/v2 v2.80.1 // indirect | ||
k8s.io/kube-openapi v0.0.0-20221012153701-172d655c2280 // indirect | ||
k8s.io/utils v0.0.0-20221107191617-1a15be271d1d // indirect | ||
sigs.k8s.io/json v0.0.0-20220713155537-f223a00ba0e2 // indirect | ||
sigs.k8s.io/structured-merge-diff/v4 v4.2.3 // indirect | ||
sigs.k8s.io/yaml v1.3.0 // indirect | ||
) |
Large diffs are not rendered by default.
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -19,8 +19,6 @@ func DockerLikeImageInfo(baseUrl string) func(image, tag string) ([]string, erro | |
baseUrl = strings.Trim(baseUrl, "/") | ||
|
||
return func(image, tag string) ([]string, error) { | ||
digests := make([]string, 0, 2) | ||
|
||
manifestsUrl := fmt.Sprintf(baseUrl+"/%s/manifests/%s", image, tag) | ||
headRsp, err := http.Head(manifestsUrl) | ||
if err != nil { | ||
|
@@ -50,9 +48,18 @@ func DockerLikeImageInfo(baseUrl string) func(image, tag string) ([]string, erro | |
req.Header.Set("Authorization", "Bearer "+token) | ||
} | ||
|
||
digests := []string{} | ||
|
||
retDigests, err := ociDigests(req) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Multiple functions mutating this request can be a bit error prone, I'd be weary of this. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yes is not the best way I agree, TBH I just extended the current code to contemplate this case, didn't want to do a bigger refactor for this knowing that we will replace it soon. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
if err != nil { | ||
return nil, fmt.Errorf("getting oci image digests from docker: %w", err) | ||
} | ||
|
||
digests = append(digests, retDigests...) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we need both OCI-formatted digests and v2 manifest digests? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I kept both to avoid breaking changes on the non oci images. |
||
|
||
// Query the manifest list digest | ||
req.Header.Set("Accept", "application/vnd.docker.distribution.manifest.list.v2+json") | ||
retDigests, err := dockerContentDigest(req) | ||
retDigests, err = dockerContentDigest(req) | ||
if err != nil { | ||
return nil, fmt.Errorf("getting manifest list from docker: %w", err) | ||
} | ||
|
@@ -63,11 +70,15 @@ func DockerLikeImageInfo(baseUrl string) func(image, tag string) ([]string, erro | |
req.Header.Set("Accept", "application/vnd.docker.distribution.manifest.v2+json") | ||
retDigests, err = dockerContentDigest(req) | ||
if err != nil { | ||
return nil, fmt.Errorf("getting manifest list from docker: %w", err) | ||
return nil, fmt.Errorf("getting manifest from docker: %w", err) | ||
} | ||
|
||
digests = append(digests, retDigests...) | ||
|
||
if len(digests) == 0 { | ||
return nil, fmt.Errorf("getting images digests for the docker image %s:%s", image, tag) | ||
} | ||
|
||
return digests, nil | ||
} | ||
} | ||
|
@@ -83,12 +94,48 @@ func dockerContentDigest(req *http.Request) ([]string, error) { | |
|
||
digest, found := resp.Header["Docker-Content-Digest"] | ||
if !found { | ||
return nil, fmt.Errorf("docker API did not return a content digest for this image") | ||
// OCI images will not respond with the list of digest | ||
return nil, nil | ||
Comment on lines
+97
to
+98
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Rather than calling There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not completely sure about it (is a bit blurry to me how this API works) so I followed the existing mechanism that was adding digest to the array. |
||
} | ||
|
||
return digest, nil | ||
} | ||
|
||
func ociDigests(req *http.Request) ([]string, error) { | ||
// https://github.com/moby/buildkit/issues/2251 | ||
acceptHeader := "application/vnd.oci.image.index.v1+json" | ||
req.Header.Set("Accept", "application/vnd.oci.image.index.v1+json") | ||
resp, err := http.DefaultClient.Do(req) | ||
if err != nil { | ||
return nil, fmt.Errorf("querying dockerhub tags endpoint: %w", err) | ||
} | ||
|
||
defer resp.Body.Close() | ||
|
||
if resp.Header.Get("Content-Type") != acceptHeader { | ||
return nil, nil | ||
} | ||
Comment on lines
+115
to
+117
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Seems like this should be an error or at least a log, wouldn't it? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. the api return different content-type event if the accept header is set to the image one. this was a quick way to filter this responses, but I didn't want to log error since this is expected and logging this will pollute the logs. |
||
|
||
body, err := io.ReadAll(resp.Body) | ||
if err != nil { | ||
return nil, fmt.Errorf("reading response body: %w", err) | ||
} | ||
|
||
indexResp := &IndexResp{} | ||
|
||
if err := json.Unmarshal(body, indexResp); err != nil { | ||
return nil, fmt.Errorf("decoding response body: %w", err) | ||
} | ||
|
||
digests := []string{} | ||
|
||
for _, manifest := range indexResp.Manifests { | ||
digests = append(digests, manifest.Digest) | ||
} | ||
|
||
return digests, nil | ||
} | ||
|
||
func authUrl(wwwAuthenticate string) (*url.URL, error) { | ||
if !strings.HasPrefix(wwwAuthenticate, "Bearer ") { | ||
return nil, fmt.Errorf("www-authenticate header lacks Bearer prefix") | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,28 @@ | ||
package docker_test | ||
|
||
import ( | ||
"testing" | ||
|
||
"github.com/newrelic-forks/reroller/src/registry/docker" | ||
"github.com/stretchr/testify/require" | ||
) | ||
|
||
func Test_Dockerhub(t *testing.T) { | ||
infofunc := docker.DockerLikeImageInfo(docker.BaseUrl) | ||
|
||
digests, err := infofunc("newrelic/infrastructure-bundle", "1.0.0") | ||
require.NoError(t, err) | ||
require.Contains(t, digests, "sha256:7300aec653dbe8aaef96b92d2f8319a2cef8e03e99b4420ba846b46a1447ea6b") | ||
} | ||
|
||
func Test_Dockerhub_oci_image(t *testing.T) { | ||
infofunc := docker.DockerLikeImageInfo(docker.BaseUrl) | ||
|
||
// test buildkit image https://github.com/moby/buildkit/issues/2251 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could we split this to a second test? That way we will know which one is failing. |
||
digests, err := infofunc("newrelic/infrastructure-bundle", "3.1.3") | ||
require.NoError(t, err) | ||
require.Contains(t, digests, "sha256:d52446ef1513dea9f5928937a268afbf8583e2877b53242345505e8b7eaca78d") | ||
require.Contains(t, digests, "sha256:500ea42793487b8251d01047d843f971590da91dd6ca257c913e04adbd9e802e") | ||
require.Contains(t, digests, "sha256:defe972d81b71f52a3c4fa30b5c6c192c950da7e937516d249429f7900d5f174") | ||
|
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,29 @@ | ||
package docker | ||
|
||
// OCI index schema | ||
type IndexResp struct { | ||
MediaType string `json:"mediaType"` | ||
SchemaVersion int `json:"schemaVersion"` | ||
Manifests []Manifests `json:"manifests"` | ||
} | ||
type Platform struct { | ||
Architecture string `json:"architecture"` | ||
Os string `json:"os"` | ||
} | ||
type Platform0 struct { | ||
Architecture string `json:"architecture"` | ||
Os string `json:"os"` | ||
Variant string `json:"variant"` | ||
} | ||
type Annotations struct { | ||
VndDockerReferenceDigest string `json:"vnd.docker.reference.digest"` | ||
VndDockerReferenceType string `json:"vnd.docker.reference.type"` | ||
} | ||
type Manifests struct { | ||
MediaType string `json:"mediaType"` | ||
Digest string `json:"digest"` | ||
Size int `json:"size"` | ||
Platform Platform `json:"platform,omitempty"` | ||
Platform0 Platform0 `json:"platform,omitempty"` | ||
Annotations Annotations `json:"annotations,omitempty"` | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
GitHub has an OCI registry per repo. Instead of publishing to Roobre's registry, we can still publish to the GitHub one.
This way you will neither need to upload the image to your account nor we will not need a secret to upload to new relic's registry. We will be able to archive this repository easily too.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh this is a very good one, I will do that.