feat(controller)!: Key-only artifacts. Fixes argoproj#3184 (argoproj#…

…4618)

Signed-off-by: Alex Collins <[email protected]>
Co-authored-by: Saravanan Balasubramanian <[email protected]>

api/jsonschema/schema.json

@@ -3340,7 +3340,6 @@
  }
  },
  "required": [
- "bucket",
  "key"
  ],
  "type": "object"
@@ -3478,7 +3477,6 @@
  }
  },
  "required": [
- "addresses",
  "path"
  ],
  "type": "object"
@@ -3936,10 +3934,6 @@
  }
  },
  "required": [
- "endpoint",
- "bucket",
- "accessKeySecret",
- "secretKeySecret",
  "key"
  ],
  "type": "object"
@@ -4202,10 +4196,6 @@
  }
  },
  "required": [
- "endpoint",
- "bucket",
- "accessKeySecret",
- "secretKeySecret",
  "key"
  ],
  "type": "object"

api/openapi-spec/swagger.json

@@ -6741,7 +6741,6 @@
  "description": "GCSArtifact is the location of a GCS artifact",
  "type": "object",
  "required": [
- "bucket",
  "key"
  ],
  "properties": {
@@ -6848,7 +6847,6 @@
  "description": "HDFSArtifact is the location of an HDFS artifact",
  "type": "object",
  "required": [
- "addresses",
  "path"
  ],
  "properties": {
@@ -7329,10 +7327,6 @@
  "description": "OSSArtifact is the location of an Alibaba Cloud OSS artifact",
  "type": "object",
  "required": [
- "endpoint",
- "bucket",
- "accessKeySecret",
- "secretKeySecret",
  "key"
  ],
  "properties": {
@@ -7575,10 +7569,6 @@
  "description": "S3Artifact is the location of an S3 artifact",
  "type": "object",
  "required": [
- "endpoint",
- "bucket",
- "accessKeySecret",
- "secretKeySecret",
  "key"
  ],
  "properties": {

config/config.go

@@ -2,12 +2,14 @@ package config
 
 import (
  "fmt"
+ "path"
 
  apiv1 "k8s.io/api/core/v1"
  metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
  "k8s.io/apimachinery/pkg/labels"
 
  wfv1 "github.com/argoproj/argo/pkg/apis/workflow/v1alpha1"
+ "github.com/argoproj/argo/workflow/common"
 )
 
 var EmptyConfigFunc = func() interface{} { return &Config{} }
@@ -138,6 +140,41 @@ func (a *ArtifactRepository) IsArchiveLogs() bool {
  return a != nil && a.ArchiveLogs != nil && *a.ArchiveLogs
 }
 
+type ArtifactRepositoryType interface {
+ IntoArtifactLocation(l *wfv1.ArtifactLocation)
+}
+
+func (a *ArtifactRepository) Get() ArtifactRepositoryType {
+ if a == nil {
+ return nil
+ } else if a.Artifactory != nil {
+ return a.Artifactory
+ } else if a.GCS != nil {
+ return a.GCS
+ } else if a.HDFS != nil {
+ return a.HDFS
+ } else if a.OSS != nil {
+ return a.OSS
+ } else if a.S3 != nil {
+ return a.S3
+ }
+ return nil
+}
+
+// ToArtifactLocation returns the artifact location set with default template key:
+// key = `{{workflow.name}}/{{pod.name}}`
+func (a *ArtifactRepository) ToArtifactLocation() *wfv1.ArtifactLocation {
+ if a == nil {
+ return nil
+ }
+ l := &wfv1.ArtifactLocation{ArchiveLogs: a.ArchiveLogs}
+ v := a.Get()
+ if v != nil {
+ v.IntoArtifactLocation(l)
+ }
+ return l
+}
+
 type PersistConfig struct {
  NodeStatusOffload bool `json:"nodeStatusOffLoad,omitempty"`
  // Archive workflows to persistence.
@@ -212,6 +249,14 @@ type S3ArtifactRepository struct {
  KeyPrefix string `json:"keyPrefix,omitempty"`
 }
 
+func (r *S3ArtifactRepository) IntoArtifactLocation(l *wfv1.ArtifactLocation) {
+ k := r.KeyFormat
+ if k == "" {
+ k = path.Join(r.KeyPrefix, common.DefaultArchivePattern)
+ }
+ l.S3 = &wfv1.S3Artifact{S3Bucket: r.S3Bucket, Key: k}
+}
+
 // OSSArtifactRepository defines the controller configuration for an OSS artifact repository
 type OSSArtifactRepository struct {
  wfv1.OSSBucket `json:",inline"`
@@ -220,6 +265,14 @@ type OSSArtifactRepository struct {
  KeyFormat string `json:"keyFormat,omitempty"`
 }
 
+func (r *OSSArtifactRepository) IntoArtifactLocation(l *wfv1.ArtifactLocation) {
+ k := r.KeyFormat
+ if k == "" {
+ k = common.DefaultArchivePattern
+ }
+ l.OSS = &wfv1.OSSArtifact{OSSBucket: r.OSSBucket, Key: k}
+}
+
 // GCSArtifactRepository defines the controller configuration for a GCS artifact repository
 type GCSArtifactRepository struct {
  wfv1.GCSBucket `json:",inline"`
@@ -228,13 +281,30 @@ type GCSArtifactRepository struct {
  KeyFormat string `json:"keyFormat,omitempty"`
 }
 
+func (r *GCSArtifactRepository) IntoArtifactLocation(l *wfv1.ArtifactLocation) {
+ k := r.KeyFormat
+ if k == "" {
+ k = common.DefaultArchivePattern
+ }
+ l.GCS = &wfv1.GCSArtifact{GCSBucket: r.GCSBucket, Key: k}
+}
+
 // ArtifactoryArtifactRepository defines the controller configuration for an artifactory artifact repository
 type ArtifactoryArtifactRepository struct {
  wfv1.ArtifactoryAuth `json:",inline"`
  // RepoURL is the url for artifactory repo.
  RepoURL string `json:"repoURL,omitempty"`
 }
 
+func (r *ArtifactoryArtifactRepository) IntoArtifactLocation(l *wfv1.ArtifactLocation) {
+ u := ""
+ if r.RepoURL != "" {
+ u = r.RepoURL + "/"
+ }
+ u = fmt.Sprintf("%s%s", u, common.DefaultArchivePattern)
+ l.Artifactory = &wfv1.ArtifactoryArtifact{ArtifactoryAuth: r.ArtifactoryAuth, URL: u}
+}
+
 // HDFSArtifactRepository defines the controller configuration for an HDFS artifact repository
 type HDFSArtifactRepository struct {
  wfv1.HDFSConfig `json:",inline"`
@@ -246,6 +316,14 @@ type HDFSArtifactRepository struct {
  Force bool `json:"force,omitempty"`
 }
 
+func (r *HDFSArtifactRepository) IntoArtifactLocation(l *wfv1.ArtifactLocation) {
+ p := r.PathFormat
+ if p == "" {
+ p = common.DefaultArchivePattern
+ }
+ l.HDFS = &wfv1.HDFSArtifact{HDFSConfig: r.HDFSConfig, Path: p, Force: r.Force}
+}
+
 // MetricsConfig defines a config for a metrics server
 type MetricsConfig struct {
  // Enabled controls metric emission. Default is true, set "enabled: false" to turn off

config/config_test.go

@@ -7,6 +7,60 @@ import (
  "k8s.io/utils/pointer"
 )
 
+func TestArtifactRepository(t *testing.T) {
+ t.Run("Nil", func(t *testing.T) {
+ var r *ArtifactRepository
+ assert.Nil(t, r.Get())
+ l := r.ToArtifactLocation()
+ assert.Nil(t, l)
+ })
+ t.Run("ArchiveLogs", func(t *testing.T) {
+ r := &ArtifactRepository{Artifactory: &ArtifactoryArtifactRepository{}, ArchiveLogs: pointer.BoolPtr(true)}
+ l := r.ToArtifactLocation()
+ assert.Equal(t, pointer.BoolPtr(true), l.ArchiveLogs)
+ })
+ t.Run("Artifactory", func(t *testing.T) {
+ r := &ArtifactRepository{Artifactory: &ArtifactoryArtifactRepository{RepoURL: "http:https://my-repo"}}
+ assert.IsType(t, &ArtifactoryArtifactRepository{}, r.Get())
+ l := r.ToArtifactLocation()
+ if assert.NotNil(t, l.Artifactory) {
+ assert.Equal(t, "http:https://my-repo/{{workflow.name}}/{{pod.name}}", l.Artifactory.URL)
+ }
+ })
+ t.Run("GCS", func(t *testing.T) {
+ r := &ArtifactRepository{GCS: &GCSArtifactRepository{}}
+ assert.IsType(t, &GCSArtifactRepository{}, r.Get())
+ l := r.ToArtifactLocation()
+ if assert.NotNil(t, l.GCS) {
+ assert.Equal(t, "{{workflow.name}}/{{pod.name}}", l.GCS.Key)
+ }
+ })
+ t.Run("HDFS", func(t *testing.T) {
+ r := &ArtifactRepository{HDFS: &HDFSArtifactRepository{}}
+ assert.IsType(t, &HDFSArtifactRepository{}, r.Get())
+ l := r.ToArtifactLocation()
+ if assert.NotNil(t, l.HDFS) {
+ assert.Equal(t, "{{workflow.name}}/{{pod.name}}", l.HDFS.Path)
+ }
+ })
+ t.Run("OSS", func(t *testing.T) {
+ r := &ArtifactRepository{OSS: &OSSArtifactRepository{}}
+ assert.IsType(t, &OSSArtifactRepository{}, r.Get())
+ l := r.ToArtifactLocation()
+ if assert.NotNil(t, l.OSS) {
+ assert.Equal(t, "{{workflow.name}}/{{pod.name}}", l.OSS.Key)
+ }
+ })
+ t.Run("S3", func(t *testing.T) {
+ r := &ArtifactRepository{S3: &S3ArtifactRepository{KeyPrefix: "my-key-prefix"}}
+ assert.IsType(t, &S3ArtifactRepository{}, r.Get())
+ l := r.ToArtifactLocation()
+ if assert.NotNil(t, l.S3) {
+ assert.Equal(t, "my-key-prefix/{{workflow.name}}/{{pod.name}}", l.S3.Key)
+ }
+ })
+}
+
 func TestArtifactRepository_IsArchiveLogs(t *testing.T) {
  assert.False(t, (&ArtifactRepository{}).IsArchiveLogs())
  assert.False(t, (&ArtifactRepository{ArchiveLogs: pointer.BoolPtr(false)}).IsArchiveLogs())

docs/artifact-repository-ref.md

@@ -15,7 +15,7 @@ metadata:
  # if you want to use this config map by default - name it "artifact-repositories" 
  name: artifact-repositories
  annotations:
- # if you want to use a specific key, put that's key into this annotation 
+ # v3.0 and after - if you want to use a specific key, put that's key into this annotation 
  workflows.argoproj.io/default-artifact-repository: default-v1
 data:
  default-v1: |
@@ -40,4 +40,6 @@ spec:
  key: my-key # default can be set by the annotation
 ```
 
+This feature gives maximum benefit when used with [key-only artifacts](key-only-artifacts.md).
+
 Reference: [fields.md#artifactrepositoryref](fields.md#artifactrepositoryref).