From 3100dc6c92a60fafefaffcb527cbd307e74d064d Mon Sep 17 00:00:00 2001 From: jakobmoellerdev Date: Mon, 6 Jan 2025 17:55:22 +0100 Subject: [PATCH] chore: fixing various bugs and testing input by artifact set --- api/ocm/plugin/ppi/cmds/upload/put/cmd.go | 53 +++--- api/ocm/plugin/ppi/interface.go | 3 +- cmds/demoplugin/uploaders/demo.go | 3 +- cmds/jfrogplugin/config/config.go | 21 +++ cmds/jfrogplugin/main.go | 61 +------ cmds/jfrogplugin/main_test.go | 201 +++++++++++++++++++++ cmds/jfrogplugin/ppi/ppi.go | 63 +++++++ cmds/jfrogplugin/suite_test.go | 13 ++ cmds/jfrogplugin/testhelper/env.go | 81 +++++++++ cmds/jfrogplugin/uploaders/helm/convert.go | 7 +- cmds/jfrogplugin/uploaders/helm/helm.go | 92 +++++----- cmds/jfrogplugin/uploaders/helm/reindex.go | 2 +- cmds/jfrogplugin/uploaders/helm/upload.go | 56 +++--- 13 files changed, 494 insertions(+), 162 deletions(-) create mode 100644 cmds/jfrogplugin/config/config.go create mode 100644 cmds/jfrogplugin/main_test.go create mode 100644 cmds/jfrogplugin/ppi/ppi.go create mode 100644 cmds/jfrogplugin/suite_test.go create mode 100644 cmds/jfrogplugin/testhelper/env.go diff --git a/api/ocm/plugin/ppi/cmds/upload/put/cmd.go b/api/ocm/plugin/ppi/cmds/upload/put/cmd.go index 0497713d14..dfcef8844f 100644 --- a/api/ocm/plugin/ppi/cmds/upload/put/cmd.go +++ b/api/ocm/plugin/ppi/cmds/upload/put/cmd.go @@ -3,8 +3,6 @@ package put import ( "encoding/json" "fmt" - "io" - "os" "github.com/mandelsoft/goutils/errors" "github.com/spf13/cobra" @@ -15,7 +13,6 @@ import ( "ocm.software/ocm/api/ocm/plugin/ppi" "ocm.software/ocm/api/ocm/plugin/ppi/cmds/common" "ocm.software/ocm/api/utils/cobrautils/flag" - "ocm.software/ocm/api/utils/iotools" "ocm.software/ocm/api/utils/runtime" ) @@ -82,10 +79,10 @@ func (o *Options) Complete(args []string) error { return nil } -func Command(p ppi.Plugin, cmd *cobra.Command, opts *Options) error { - spec, err := p.DecodeUploadTargetSpecification(opts.Specification) - if err != nil { - return fmt.Errorf("target specification: %w", err) +func Command(p ppi.Plugin, cmd *cobra.Command, opts *Options) (err error) { + var spec ppi.UploadTargetSpec + if spec, err = p.DecodeUploadTargetSpecification(opts.Specification); err != nil { + return fmt.Errorf("error decoding upload target specification: %w", err) } u := p.GetUploader(opts.Name) @@ -93,36 +90,28 @@ func Command(p ppi.Plugin, cmd *cobra.Command, opts *Options) error { return errors.ErrNotFound(descriptor.KIND_UPLOADER, fmt.Sprintf("%s:%s", opts.ArtifactType, opts.MediaType)) } - reader := io.Reader(os.Stdin) - // if we are not size aware, buffer the file to avoid OOM. - if opts.Digest == "" { - tmp, err := os.CreateTemp("", "ocm-upload-") - if err != nil { - return fmt.Errorf("failed to create temporary file to buffer access data: %w", err) - } - defer func() { - tmp.Close() - os.Remove(tmp.Name()) - }() - digestReader := iotools.NewDefaultDigestReader(reader) - if _, err := io.Copy(tmp, digestReader); err != nil { - return fmt.Errorf("failed to read from stdin: %w", err) - } - reader = tmp - opts.Digest = digestReader.Digest().String() - } - - h, err := u.Upload(p, opts.ArtifactType, opts.MediaType, opts.Hint, opts.Digest, spec, opts.Credentials, reader) - if err != nil { + reader := cmd.InOrStdin() + + var provider ppi.AccessSpecProvider + if provider, err = u.Upload( + cmd.Context(), + p, + opts.ArtifactType, + opts.MediaType, + opts.Hint, + opts.Digest, + spec, + opts.Credentials, + reader, + ); err != nil { return fmt.Errorf("upload failed: %w", err) } - acc := h() + acc := provider() - data, err := json.Marshal(acc) - if err == nil { + var data []byte + if data, err = json.Marshal(acc); err == nil { cmd.Printf("%s\n", string(data)) } - return err } diff --git a/api/ocm/plugin/ppi/interface.go b/api/ocm/plugin/ppi/interface.go index a1949b2a12..27f9048ee5 100644 --- a/api/ocm/plugin/ppi/interface.go +++ b/api/ocm/plugin/ppi/interface.go @@ -1,6 +1,7 @@ package ppi import ( + "context" "encoding/json" "io" @@ -113,7 +114,7 @@ type Uploader interface { Description() string ValidateSpecification(p Plugin, spec UploadTargetSpec) (info *UploadTargetSpecInfo, err error) - Upload(p Plugin, arttype, mediatype, hint, digest string, spec UploadTargetSpec, creds credentials.Credentials, reader io.Reader) (AccessSpecProvider, error) + Upload(ctx context.Context, p Plugin, arttype, mediatype, hint, digest string, spec UploadTargetSpec, creds credentials.Credentials, reader io.Reader) (AccessSpecProvider, error) } type UploadTargetSpec = runtime.TypedObject diff --git a/cmds/demoplugin/uploaders/demo.go b/cmds/demoplugin/uploaders/demo.go index b2bb80114a..3073bc0fb6 100644 --- a/cmds/demoplugin/uploaders/demo.go +++ b/cmds/demoplugin/uploaders/demo.go @@ -1,6 +1,7 @@ package uploaders import ( + "context" "fmt" "io" "os" @@ -72,7 +73,7 @@ func (a *Uploader) ValidateSpecification(_ ppi.Plugin, spec ppi.UploadTargetSpec return &info, nil } -func (a *Uploader) Upload(p ppi.Plugin, arttype, mediatype, hint, digest string, spec ppi.UploadTargetSpec, creds credentials.Credentials, reader io.Reader) (ppi.AccessSpecProvider, error) { +func (a *Uploader) Upload(_ context.Context, p ppi.Plugin, arttype, mediatype, hint, digest string, spec ppi.UploadTargetSpec, creds credentials.Credentials, reader io.Reader) (ppi.AccessSpecProvider, error) { var file *os.File var err error diff --git a/cmds/jfrogplugin/config/config.go b/cmds/jfrogplugin/config/config.go new file mode 100644 index 0000000000..27d735f880 --- /dev/null +++ b/cmds/jfrogplugin/config/config.go @@ -0,0 +1,21 @@ +package config + +import ( + "encoding/json" + "fmt" +) + +type Config struct { +} + +// GetConfig returns the config from the raw json message. +// any return is required for the plugin interface. +func GetConfig(raw json.RawMessage) (interface{}, error) { + var cfg Config + + if err := json.Unmarshal(raw, &cfg); err != nil { + return nil, fmt.Errorf("could not get config: %w", err) + } + return &cfg, nil + +} diff --git a/cmds/jfrogplugin/main.go b/cmds/jfrogplugin/main.go index 5bc535fdbd..ab5cdb49ca 100644 --- a/cmds/jfrogplugin/main.go +++ b/cmds/jfrogplugin/main.go @@ -1,70 +1,21 @@ package main import ( - "encoding/json" "fmt" "os" - "strconv" - "ocm.software/ocm/api/config" - "ocm.software/ocm/api/ocm/extensions/artifacttypes" - "ocm.software/ocm/api/ocm/extensions/blobhandler" - "ocm.software/ocm/api/ocm/plugin" - "ocm.software/ocm/api/ocm/plugin/ppi" "ocm.software/ocm/api/ocm/plugin/ppi/cmds" - "ocm.software/ocm/api/version" - "ocm.software/ocm/cmds/jfrogplugin/uploaders/helm" + jfrogppi "ocm.software/ocm/cmds/jfrogplugin/ppi" ) -const NAME = "jfrog" - func main() { - p := ppi.NewPlugin(NAME, version.Get().String()) - - p.SetShort(NAME + " plugin") - p.SetLong(`ALPHA GRADE plugin providing custom functions related to interacting with JFrog Repositories (e.g. Artifactory). - -This plugin is solely for interacting with JFrog Servers and cannot be used for generic repository types. -Thus, you should only consider this plugin if -- You need to use a JFrog specific API -- You cannot use any of the generic (non-jfrog) implementations. - -Examples: - -You can configure the JFrog plugin as an Uploader in an ocm config file with: - -- type: ` + fmt.Sprintf("%s.ocm.%s", plugin.KIND_UPLOADER, config.OCM_CONFIG_TYPE_SUFFIX) + ` - registrations: - - name: ` + fmt.Sprintf("%s/%s/%s", plugin.KIND_PLUGIN, NAME, helm.NAME) + ` - artifactType: ` + artifacttypes.HELM_CHART + ` - priority: 200 # must be > ` + strconv.Itoa(blobhandler.DEFAULT_BLOBHANDLER_PRIO) + ` to be used over the default handler - config: - type: ` + fmt.Sprintf("%s/%s", helm.NAME, helm.VERSION) + ` - # this is only a sample JFrog Server URL, do NOT append /artifactory - url: int.repositories.ocm.software - repository: ocm-helm-test -`) - p.SetConfigParser(GetConfig) - - u := helm.New() - if err := p.RegisterUploader(artifacttypes.HELM_CHART, "", u); err != nil { - panic(err) - } - err := cmds.NewPluginCommand(p).Execute(os.Args[1:]) + plugin, err := jfrogppi.Plugin() if err != nil { - fmt.Fprintf(os.Stderr, "error while running plugin: %v\n", err) + fmt.Fprintf(os.Stderr, "error while creating plugin: %v\n", err) os.Exit(1) } -} - -type Config struct { -} - -func GetConfig(raw json.RawMessage) (interface{}, error) { - var cfg Config - - if err := json.Unmarshal(raw, &cfg); err != nil { - return nil, fmt.Errorf("could not get config: %w", err) + if err := cmds.NewPluginCommand(plugin).Execute(os.Args[1:]); err != nil { + fmt.Fprintf(os.Stderr, "error while running plugin: %v\n", err) + os.Exit(1) } - return &cfg, nil } diff --git a/cmds/jfrogplugin/main_test.go b/cmds/jfrogplugin/main_test.go new file mode 100644 index 0000000000..f4a22868e8 --- /dev/null +++ b/cmds/jfrogplugin/main_test.go @@ -0,0 +1,201 @@ +package main_test + +import ( + "bytes" + "compress/gzip" + "encoding/json" + "fmt" + "io" + "net/http" + "net/http/httptest" + "net/url" + "os" + "path/filepath" + "strconv" + "strings" + "time" + + . "github.com/mandelsoft/goutils/testutils" + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + + "ocm.software/ocm/api/credentials" + "ocm.software/ocm/api/oci/artdesc" + "ocm.software/ocm/api/oci/extensions/repositories/artifactset" + helmaccess "ocm.software/ocm/api/ocm/extensions/accessmethods/helm" + "ocm.software/ocm/api/ocm/extensions/artifacttypes" + "ocm.software/ocm/api/ocm/plugin/ppi" + "ocm.software/ocm/api/tech/helm/loader" + "ocm.software/ocm/api/utils/runtime" + . "ocm.software/ocm/cmds/jfrogplugin/testhelper" + "ocm.software/ocm/cmds/jfrogplugin/uploaders/helm" +) + +var _ = Describe(helm.VERSIONED_NAME, func() { + var env *TestEnv + + BeforeEach(func() { + env = Must(NewTestEnv()) + DeferCleanup(env.Cleanup) + }) + + It("Upload Validate Invalid Spec", func(ctx SpecContext) { + env.CLI.Command().SetContext(ctx) + err := env.Execute("upload", "validate", "--artifactType=bla", "abc", "def") + Expect(err).To(HaveOccurred()) + Expect(err).To(MatchError(ContainSubstring("error unmarshaling JSON"))) + }) + + It("Validate Spec OK and empty", func(ctx SpecContext) { + env.CLI.Command().SetContext(ctx) + + uploadSpec := &helm.JFrogHelmUploaderSpec{ + ObjectVersionedType: runtime.ObjectVersionedType{Type: helm.VERSIONED_NAME}, + } + + Expect(env.Execute( + "upload", "validate", "--artifactType", artifacttypes.HELM_CHART, + helm.NAME, + string(Must(json.Marshal(uploadSpec))), + )).To(Succeed()) + + Expect(env.CLI.GetOutput()).To(Not(BeEmpty())) + var info ppi.UploadTargetSpecInfo + Expect(json.Unmarshal(env.CLI.GetOutput(), &info)).To(Succeed()) + Expect(info.ConsumerId.Type()).To(Equal(helm.NAME)) + }) + + It("Validate Upload Spec OK with full identity based on Artifact Set containing OCI Image", func(ctx SpecContext) { + env.CLI.Command().SetContext(ctx) + + purl := Must(url.Parse("https://ocm.software:5501/my-artifactory")) + uploadSpec := &helm.JFrogHelmUploaderSpec{ + ObjectVersionedType: runtime.ObjectVersionedType{Type: helm.VERSIONED_NAME}, + URL: purl.String(), + Repository: "my-repo", + } + + Expect(env.Execute("upload", "validate", + "--artifactType", artifacttypes.HELM_CHART, + helm.NAME, + string(Must(json.Marshal(uploadSpec))), + )).To(Succeed()) + + var info ppi.UploadTargetSpecInfo + output := env.CLI.GetOutput() + Expect(output).To(Not(BeEmpty())) + Expect(json.Unmarshal(output, &info)).To(Succeed()) + + Expect(info.ConsumerId.Type()).To(Equal(helm.NAME)) + Expect(info.ConsumerId.Match(credentials.ConsumerIdentity{ + helm.ID_TYPE: helm.NAME, + helm.ID_HOSTNAME: purl.Hostname(), + helm.ID_PORT: purl.Port(), + helm.ID_REPOSITORY: "my-repo", + })).To(BeTrue(), "the identity should contain all attributes relevant to"+ + " match the correct repository for a resource transfer") + }) + + It("Upload Artifact Set to Server", func(ctx SpecContext) { + env.CLI.Command().SetContext(ctx) + + user, pass := "mocked", "mocked" + testDataPath := Must(filepath.Abs("../../api/ocm/extensions/download/handlers/helm/testdata/test-chart-oci-artifact.tgz")) + testDataFile := Must(os.OpenFile(testDataPath, os.O_RDONLY, 0o400)) + DeferCleanup(testDataFile.Close) + testData := Must(io.ReadAll(testDataFile)) + + // explicit variable for in-scope reference to server + var server *httptest.Server + reindexedAfterUpload := false + server = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if strings.Contains(r.URL.String(), "reindex") { + w.WriteHeader(http.StatusOK) + reindexedAfterUpload = true + return + } + + if r.Method != http.MethodPut { + http.Error(w, fmt.Sprintf("expected %s request, got %s", http.MethodPut, r.Method), http.StatusBadRequest) + } + + u, p, ok := r.BasicAuth() + if !ok { + http.Error(w, fmt.Sprintf("expected basic auth header, got %s,%s but expected %s,%s", u, p, user, pass), http.StatusBadRequest) + } + + data := Must(io.ReadAll(r.Body)) + unzipped, err := gzip.NewReader(bytes.NewReader(data)) + if err != nil { + http.Error(w, fmt.Sprintf("failed to unzip data: %v", err), http.StatusBadRequest) + } + var buf bytes.Buffer + io.Copy(&buf, unzipped) + unzipped.Close() + bufBytes := buf.Bytes() + var compressed bytes.Buffer + writer := gzip.NewWriter(&compressed) + io.Copy(writer, bytes.NewReader(bufBytes)) + writer.Close() + data = compressed.Bytes() + + chart, err := loader.LoadArchive(bytes.NewReader(data)) + if err != nil { + http.Error(w, fmt.Sprintf("failed to load chart: %s", err.Error()), http.StatusBadRequest) + } + + resData, err := json.Marshal(&helm.ArtifactoryUploadResponse{ + DownloadUri: fmt.Sprintf("%s/my-repo/%s-%s.tgz", server.URL, chart.Name(), chart.Metadata.Version), + Path: "/mocked/chart.tgz", + CreatedBy: "mocked", + Created: time.Now().Format(time.RFC3339), + Repo: "my-repo", + MimeType: helm.MEDIA_TYPE, + Checksums: helm.ArtifactoryUploadChecksums{ + Sha256: r.Header.Get("X-Checksum-Sha256"), + }, + Size: strconv.Itoa(len(data)), + }) + if err != nil { + http.Error(w, fmt.Sprintf("failed to marshal response: %v", err), http.StatusInternalServerError) + } + + // mimick the upload response + w.WriteHeader(http.StatusCreated) + if _, err := io.Copy(w, bytes.NewReader(resData)); err != nil { + Fail(fmt.Sprintf("failed to write response: %v", err)) + } + })) + DeferCleanup(server.Close) + + purl := Must(helm.ParseURLAllowNoScheme(server.URL)) + uploadSpec := &helm.JFrogHelmUploaderSpec{ + ObjectVersionedType: runtime.ObjectVersionedType{Type: helm.VERSIONED_NAME}, + URL: purl.String(), + Repository: "my-repo", + } + + env.CLI.SetInput(io.NopCloser(bytes.NewReader(testData))) + Expect(env.Execute("upload", "put", + "--artifactType", artifacttypes.HELM_CHART, + "--mediaType", artifactset.MediaType(artdesc.MediaTypeImageManifest), + "--credentials", string(Must(json.Marshal(credentials.DirectCredentials{ + credentials.ATTR_USERNAME: user, + credentials.ATTR_PASSWORD: pass, + }))), + helm.NAME, + string(Must(json.Marshal(uploadSpec)))), + ).To(Succeed()) + + var spec helmaccess.AccessSpec + Expect(json.Unmarshal(env.GetOutput(), &spec)).To(Succeed()) + Expect(spec).To(Not(BeNil())) + Expect(spec.HelmRepository).To(Equal(fmt.Sprintf("%s/artifactory/api/helm/%s", server.URL, "my-repo"))) + Expect(spec.HelmChart).To(ContainSubstring(":"), "helm chart is separated with version") + splitChart := strings.Split(spec.HelmChart, ":") + Expect(splitChart).To(HaveLen(2), "helm chart is separated with version") + Expect(splitChart[0]).To(Equal("test-chart"), "the chart name should be test-chart") + Expect(splitChart[1]).To(Equal("0.1.0"), "the chart version should be 0.1.0") + Expect(reindexedAfterUpload).To(BeTrue(), "the server should have been reindexed after the upload") + }) +}) diff --git a/cmds/jfrogplugin/ppi/ppi.go b/cmds/jfrogplugin/ppi/ppi.go new file mode 100644 index 0000000000..3ce9018181 --- /dev/null +++ b/cmds/jfrogplugin/ppi/ppi.go @@ -0,0 +1,63 @@ +package ppi + +import ( + "fmt" + "strconv" + + ocmconfig "ocm.software/ocm/api/config" + "ocm.software/ocm/api/ocm/extensions/artifacttypes" + "ocm.software/ocm/api/ocm/extensions/blobhandler" + "ocm.software/ocm/api/ocm/plugin" + "ocm.software/ocm/api/ocm/plugin/ppi" + "ocm.software/ocm/api/version" + "ocm.software/ocm/cmds/jfrogplugin/config" + "ocm.software/ocm/cmds/jfrogplugin/uploaders/helm" +) + +const NAME = "jfrog" + +func Plugin() (ppi.Plugin, error) { + p := ppi.NewPlugin(NAME, version.Get().String()) + + p.SetShort(NAME + " plugin") + p.SetLong(`ALPHA GRADE plugin providing custom functions related to interacting with JFrog Repositories (e.g. Artifactory). + +This plugin is solely for interacting with JFrog Servers and cannot be used for generic repository types. +Thus, you should only consider this plugin if +- You need to use a JFrog specific API +- You cannot use any of the generic (non-jfrog) implementations. + +If given an OCI Artifact Set (for example by using it on a resource with a Helm Chart backed by an OCI registry), +it will do a best effort conversion to a normal helm chart and upload that in its stead. Note that this conversion +is not perfect however, since the Upload will inevitably strip provenance information from the chart. +This can lead to unintended side effects such as +- Having the wrong digest in the resource access +- Losing the ability to convert back to an OCI artifact set without changing digests and losing provenance information. +This means that effectively you should try to migrate to pure OCI registries instead of JFrog HELM repositories as soon +as possible (this uploader is just a stop gap). + +Examples: + +You can configure the JFrog plugin as an Uploader in an ocm config file with: + +- type: ` + fmt.Sprintf("%s.ocm.%s", plugin.KIND_UPLOADER, ocmconfig.OCM_CONFIG_TYPE_SUFFIX) + ` + registrations: + - name: ` + fmt.Sprintf("%s/%s/%s", plugin.KIND_PLUGIN, NAME, helm.NAME) + ` + artifactType: ` + artifacttypes.HELM_CHART + ` + priority: 200 # must be > ` + strconv.Itoa(blobhandler.DEFAULT_BLOBHANDLER_PRIO) + ` to be used over the default handler + config: + type: ` + fmt.Sprintf("%s/%s", helm.NAME, helm.VERSION) + ` + # this is only a sample JFrog Server URL, do NOT append /artifactory + url: int.repositories.ocm.software + repository: ocm-helm-test +`) + p.SetConfigParser(config.GetConfig) + p.ForwardLogging(true) + + u := helm.New() + if err := p.RegisterUploader(artifacttypes.HELM_CHART, "", u); err != nil { + return nil, err + } + + return p, nil +} diff --git a/cmds/jfrogplugin/suite_test.go b/cmds/jfrogplugin/suite_test.go new file mode 100644 index 0000000000..27381dfaa8 --- /dev/null +++ b/cmds/jfrogplugin/suite_test.go @@ -0,0 +1,13 @@ +package main_test + +import ( + "testing" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +func TestConfig(t *testing.T) { + RegisterFailHandler(Fail) + RunSpecs(t, "JFrog Helm uploader plugin tests") +} diff --git a/cmds/jfrogplugin/testhelper/env.go b/cmds/jfrogplugin/testhelper/env.go new file mode 100644 index 0000000000..ce1c8ef80d --- /dev/null +++ b/cmds/jfrogplugin/testhelper/env.go @@ -0,0 +1,81 @@ +package testhelper + +import ( + "bytes" + "io" + + "gopkg.in/yaml.v3" + + "ocm.software/ocm/api/helper/builder" + "ocm.software/ocm/api/helper/env" + "ocm.software/ocm/api/ocm/plugin/ppi" + "ocm.software/ocm/api/ocm/plugin/ppi/cmds" + "ocm.software/ocm/cmds/jfrogplugin/config" + jfrogppi "ocm.software/ocm/cmds/jfrogplugin/ppi" +) + +type CLI struct { + ppi.Plugin + *cmds.PluginCommand + + config string + + output *bytes.Buffer +} + +func NewCLI() (*CLI, error) { + plugin, err := jfrogppi.Plugin() + if err != nil { + return nil, err + } + cmd := cmds.NewPluginCommand(plugin) + cli := &CLI{Plugin: plugin, PluginCommand: cmd, output: &bytes.Buffer{}} + cmd.Command().SetOut(cli.output) + return cli, nil +} + +func (cli *CLI) Execute(args ...string) error { + cli.output.Reset() + if cli.config != "" { + args = append(args, "--config", cli.config) + } + return cli.PluginCommand.Execute(args) +} + +func (cli *CLI) SetConfig(cfg *config.Config) error { + var data bytes.Buffer + if err := yaml.NewEncoder(&data).Encode(cfg); err != nil { + return err + } + + cli.config = data.String() + + return nil +} + +func (cli *CLI) GetOutput() []byte { + return cli.output.Bytes() +} + +func (cli *CLI) SetInput(data io.Reader) { + cli.PluginCommand.Command().SetIn(data) +} + +type TestEnv struct { + *builder.Builder + *CLI +} + +func NewTestEnv(opts ...env.Option) (*TestEnv, error) { + b := builder.NewBuilder(opts...) + + cli, err := NewCLI() + if err != nil { + return nil, err + } + + return &TestEnv{ + Builder: b, + CLI: cli, + }, nil +} diff --git a/cmds/jfrogplugin/uploaders/helm/convert.go b/cmds/jfrogplugin/uploaders/helm/convert.go index ab9e5fbe81..fedcf2ce90 100644 --- a/cmds/jfrogplugin/uploaders/helm/convert.go +++ b/cmds/jfrogplugin/uploaders/helm/convert.go @@ -11,7 +11,12 @@ import ( "ocm.software/ocm/api/utils/accessobj" ) -func ConvertArtifactSetHelmChartToPlainTGZChart(reader io.Reader) (_ io.ReadCloser, _ string, err error) { +// ConvertArtifactSetWithOCIImageHelmChartToPlainTGZChart converts an artifact set with a single layer helm OCI image to a plain tgz chart. +// Note that this transformation is not completely reversible because an OCI artifact contains provenance data, while +// a plain tgz chart does not. +// This means converting back from a signed tgz chart to an OCI image will lose the provenance data, and also change digests. +// The returned digest is the digest of the tgz chart. +func ConvertArtifactSetWithOCIImageHelmChartToPlainTGZChart(reader io.Reader) (_ io.ReadCloser, _ string, err error) { set, err := artifactset.Open(accessobj.ACC_READONLY, "", 0, accessio.Reader(io.NopCloser(reader))) if err != nil { return nil, "", fmt.Errorf("failed to open helm OCI artifact as artifact set: %w", err) diff --git a/cmds/jfrogplugin/uploaders/helm/helm.go b/cmds/jfrogplugin/uploaders/helm/helm.go index a2d098c55b..d71290af8a 100644 --- a/cmds/jfrogplugin/uploaders/helm/helm.go +++ b/cmds/jfrogplugin/uploaders/helm/helm.go @@ -11,8 +11,6 @@ import ( "strings" "time" - "github.com/containerd/containerd/reference" - "ocm.software/ocm/api/credentials" "ocm.software/ocm/api/credentials/cpi" "ocm.software/ocm/api/credentials/identity/hostpath" @@ -26,11 +24,21 @@ import ( ) const ( + // MEDIA_TYPE is the media type of the HELM Chart artifact as tgz. + // It is the definitive format for JFrog Uploads + MEDIA_TYPE = helm.ChartMediaType + + // NAME of the Uploader and the Configuration NAME = "JFrogHelm" // VERSION of the Uploader TODO Increment once stable VERSION = "v1alpha1" + // VERSIONED_NAME is the name of the Uploader including the version + VERSIONED_NAME = NAME + runtime.VersionSeparator + VERSION + + // ID_TYPE is the type of the JFrog Artifactory credentials + ID_TYPE = cpi.ID_TYPE // ID_HOSTNAME is the hostname of the artifactory server to upload to ID_HOSTNAME = hostpath.ID_HOSTNAME // ID_PORT is the port of the artifactory server to upload to @@ -84,7 +92,7 @@ func init() { if err != nil { panic(err) } - types = ppi.UploadFormats{NAME + runtime.VersionSeparator + VERSION: decoder} + types = ppi.UploadFormats{VERSIONED_NAME: decoder} } func (a *Uploader) Decoders() ppi.UploadFormats { @@ -99,9 +107,14 @@ type Uploader struct { var _ ppi.Uploader = (*Uploader)(nil) func New() ppi.Uploader { + client := &http.Client{} + // we do not want to double compress helm tgz files + transport := http.DefaultTransport.(*http.Transport).Clone() + transport.DisableCompression = true + client.Transport = transport return &Uploader{ UploaderBase: ppi.MustNewUploaderBase(NAME, "upload artifacts to JFrog HELM repositories by using the JFrog REST API."), - Client: http.DefaultClient, + Client: client, } } @@ -127,7 +140,14 @@ func (a *Uploader) ValidateSpecification(_ ppi.Plugin, spec ppi.UploadTargetSpec // 3. creating a request respecting the passed credentials based on SetHeadersFromCredentials // 4. uploading the passed blob as is (expected to be a tgz byte stream) // 5. intepreting the JFrog API response, and converting it from ArtifactoryUploadResponse to ppi.AccessSpec -func (a *Uploader) Upload(_ ppi.Plugin, arttype, mediaType, hint, digest string, targetSpec ppi.UploadTargetSpec, creds credentials.Credentials, reader io.Reader) (ppi.AccessSpecProvider, error) { +func (a *Uploader) Upload( + ctx context.Context, + p ppi.Plugin, + arttype, mediaType, _, digest string, + targetSpec ppi.UploadTargetSpec, + creds credentials.Credentials, + reader io.Reader, +) (ppi.AccessSpecProvider, error) { if arttype != artifacttypes.HELM_CHART { return nil, fmt.Errorf("unsupported artifact type %s", arttype) } @@ -140,35 +160,36 @@ func (a *Uploader) Upload(_ ppi.Plugin, arttype, mediaType, hint, digest string, switch mediaType { case helm.ChartMediaType: // if it is a native chart tgz we can pass it on as is - var buf bytes.Buffer - chart, err := loader.LoadArchive(io.TeeReader(reader, &buf)) - if err != nil { - return nil, fmt.Errorf("failed to load chart: %w", err) - } - spec.Name = chart.Metadata.Name - spec.Version = chart.Metadata.Version - reader = &buf case artifactset.MediaType(artdesc.MediaTypeImageManifest): // if we have an artifact set (ocm custom version of index + locally colocated blobs as files, we need - // to translate it. + // to translate it. This translation is not perfect because the ociArtifactDigest that might be + // generated in OCM is not the same as the one that is used within Artifactory, but Uploaders + // do not have a way of providing back digest information to the caller. + // TODO: At some point consider this for a plugin rework. var err error - if reader, digest, err = ConvertArtifactSetHelmChartToPlainTGZChart(reader); err != nil { + if reader, digest, err = ConvertArtifactSetWithOCIImageHelmChartToPlainTGZChart(reader); err != nil { return nil, fmt.Errorf("failed to convert OCI Helm Chart to plain TGZ: %w", err) } default: return nil, fmt.Errorf("unsupported media type %s", mediaType) } - if err := EnsureSpecWithHelpFromHint(spec, hint); err != nil { - return nil, fmt.Errorf("could not ensure spec to be ready for upload: %w", err) + var buf bytes.Buffer + chart, err := loader.LoadArchive(io.TeeReader(reader, &buf)) + if err != nil { + return nil, fmt.Errorf("failed to load chart: %w", err) } + spec.Name = chart.Metadata.Name + spec.Version = chart.Metadata.Version + reader = &buf + // now based on the chart and repository we can upload it to the correct location. targetURL, err := ConvertTargetSpecToHelmUploadURL(spec) if err != nil { return nil, fmt.Errorf("failed to convert target spec to URL: %w", err) } - ctx, cancel := context.WithTimeout(context.TODO(), spec.GetTimeout()) + ctx, cancel := context.WithTimeout(ctx, spec.GetTimeout()) defer cancel() access, err := Upload(ctx, reader, a.Client, targetURL, creds, digest) @@ -190,7 +211,7 @@ func (a *Uploader) Upload(_ ppi.Plugin, arttype, mediaType, hint, digest string, // in the library to identify the correct credentials that need to // be passed to it. func ConvertTargetSpecToInfo(spec *JFrogHelmUploaderSpec) (*ppi.UploadTargetSpecInfo, error) { - purl, err := parseURLAllowNoScheme(spec.URL) + purl, err := ParseURLAllowNoScheme(spec.URL) if err != nil { return nil, fmt.Errorf("failed to parse URL: %w", err) } @@ -200,7 +221,7 @@ func ConvertTargetSpecToInfo(spec *JFrogHelmUploaderSpec) (*ppi.UploadTargetSpec // By default, we identify an artifactory repository as a combination // of Host & Repository info.ConsumerId = credentials.ConsumerIdentity{ - cpi.ID_TYPE: NAME, + ID_TYPE: NAME, ID_HOSTNAME: purl.Hostname(), ID_REPOSITORY: spec.Repository, } @@ -211,29 +232,6 @@ func ConvertTargetSpecToInfo(spec *JFrogHelmUploaderSpec) (*ppi.UploadTargetSpec return &info, nil } -// EnsureSpecWithHelpFromHint introspects the hint and fills the target spec based on it. -// It makes sure that the spec can be used to access a JFrog Artifactory HELM Repository. -func EnsureSpecWithHelpFromHint(spec *JFrogHelmUploaderSpec, hint string) error { - if refFromHint, err := reference.Parse(hint); err == nil { - if refFromHint.Digest() != "" && refFromHint.Object == "" { - return fmt.Errorf("the hint contained a valid reference but it was a digest, so it cannot be used to deduce a version of the helm chart: %s", refFromHint) - } - if spec.Version == "" { - spec.Version = refFromHint.Object - } - if spec.Name == "" { - spec.Name = path.Base(refFromHint.Locator) - } - } - if spec.Name == "" { - return fmt.Errorf("the chart name could not be deduced from the hint (%s) or the config (%s)", hint, spec) - } - if spec.Version == "" { - return fmt.Errorf("the chart version could not be deduced from the hint (%s) or the config (%s)", hint, spec) - } - return nil -} - // ConvertTargetSpecToHelmUploadURL interprets the JFrogHelmUploaderSpec into a valid REST API Endpoint URL to upload to. // It requires a valid ChartName and ChartVersion to determine the correct URL endpoint. // @@ -252,19 +250,19 @@ func EnsureSpecWithHelpFromHint(spec *JFrogHelmUploaderSpec, hint string) error // // url.URL => https://demo.jfrog.ocm.software/artifactory/my-charts/podinfo-0.0.1.tgz func ConvertTargetSpecToHelmUploadURL(spec *JFrogHelmUploaderSpec) (*url.URL, error) { - requestURL := path.Join(spec.URL, "artifactory", spec.Repository, fmt.Sprintf("%s-%s.tgz", spec.Name, spec.Version)) - requestURLParsed, err := parseURLAllowNoScheme(requestURL) + requestURLParsed, err := ParseURLAllowNoScheme(spec.URL) if err != nil { return nil, fmt.Errorf("failed to parse full request URL: %w", err) } + requestURLParsed.Path = path.Join("artifactory", spec.Repository, fmt.Sprintf("%s-%s.tgz", spec.Name, spec.Version)) return requestURLParsed, nil } -// parseURLAllowNoScheme is an adaptation / hack on url.Parse because +// ParseURLAllowNoScheme is an adaptation / hack on url.Parse because // url.Parse does not support parsing a URL without a prefixed scheme. // However, we would like to accept these kind of URLs because we default them // to "https://" out of convenience. -func parseURLAllowNoScheme(urlToParse string) (*url.URL, error) { +func ParseURLAllowNoScheme(urlToParse string) (*url.URL, error) { const dummyScheme = "dummy" if !strings.Contains(urlToParse, "://") { urlToParse = dummyScheme + "://" + urlToParse diff --git a/cmds/jfrogplugin/uploaders/helm/reindex.go b/cmds/jfrogplugin/uploaders/helm/reindex.go index 7e8f34241e..1fd1f90dd2 100644 --- a/cmds/jfrogplugin/uploaders/helm/reindex.go +++ b/cmds/jfrogplugin/uploaders/helm/reindex.go @@ -48,7 +48,7 @@ func ReindexChart(ctx context.Context, client *http.Client, artifactoryURL strin // convertToReindexURL converts the base URL and repository to a reindex URL. // see https://jfrog.com/help/r/jfrog-rest-apis/calculate-helm-chart-index for the reindex API func convertToReindexURL(baseURL string, repository string) (string, error) { - u, err := parseURLAllowNoScheme(baseURL) + u, err := ParseURLAllowNoScheme(baseURL) if err != nil { return "", fmt.Errorf("failed to parse url: %w", err) } diff --git a/cmds/jfrogplugin/uploaders/helm/upload.go b/cmds/jfrogplugin/uploaders/helm/upload.go index ff3dc91cee..5edf9e5350 100644 --- a/cmds/jfrogplugin/uploaders/helm/upload.go +++ b/cmds/jfrogplugin/uploaders/helm/upload.go @@ -36,19 +36,25 @@ func Upload( return nil, fmt.Errorf("failed to create HTTP request for upload: %w", err) } - parsedDigest, err := godigest.Parse(digest) - if err != nil { - return nil, fmt.Errorf("failed to parse digest: %w", err) - } + // if there is no digest information, we skip the digest headers. + // note that this will cause insecure uploads and should be avoided where possible + if digest != "" { + parsedDigest, err := godigest.Parse(digest) + if err != nil { + return nil, fmt.Errorf("failed to parse digest: %w", err) + } - // see https://jfrog.com/help/r/jfrog-rest-apis/deploy-artifact-by-checksum for the checksum headers - switch parsedDigest.Algorithm() { - case godigest.SHA256: - req.Header.Set("X-Checksum-Sha256", parsedDigest.Encoded()) - default: - return nil, fmt.Errorf("unsupported digest algorithm, must be %s to allow upload to jfrog: %s", godigest.SHA256, parsedDigest.Algorithm()) + // see https://jfrog.com/help/r/jfrog-rest-apis/deploy-artifact-by-checksum for the checksum headers + switch parsedDigest.Algorithm() { + case godigest.SHA256: + req.Header.Set("X-Checksum-Sha256", parsedDigest.Encoded()) + default: + return nil, fmt.Errorf("unsupported digest algorithm, must be %s to allow upload to jfrog: %s", godigest.SHA256, parsedDigest.Algorithm()) + } + req.Header.Set("X-Checksum-Deploy", "false") } - req.Header.Set("X-Checksum-Deploy", "false") + + req.Header.Set("Accept-Encoding", "gzip") SetHeadersFromCredentials(req, creds) @@ -82,19 +88,21 @@ func Upload( } type ArtifactoryUploadResponse struct { - Repo string `json:"repo,omitempty"` - Path string `json:"path,omitempty"` - Created string `json:"created,omitempty"` - CreatedBy string `json:"createdBy,omitempty"` - DownloadUri string `json:"downloadUri,omitempty"` - MimeType string `json:"mimeType,omitempty"` - Size string `json:"size,omitempty"` - Checksums struct { - Sha1 string `json:"sha1,omitempty"` - Sha256 string `json:"sha256,omitempty"` - Md5 string `json:"md5,omitempty"` - } `json:"checksums,omitempty"` - Uri string `json:"uri"` + Repo string `json:"repo,omitempty"` + Path string `json:"path,omitempty"` + Created string `json:"created,omitempty"` + CreatedBy string `json:"createdBy,omitempty"` + DownloadUri string `json:"downloadUri,omitempty"` + MimeType string `json:"mimeType,omitempty"` + Size string `json:"size,omitempty"` + Checksums ArtifactoryUploadChecksums `json:"checksums,omitempty"` + Uri string `json:"uri"` +} + +type ArtifactoryUploadChecksums struct { + Sha1 string `json:"sha1,omitempty"` + Sha256 string `json:"sha256,omitempty"` + Md5 string `json:"md5,omitempty"` } func (r *ArtifactoryUploadResponse) URL() string {