From 4aa91971531e282bcd63d827cdb1a521f607f305 Mon Sep 17 00:00:00 2001 From: Sambhav Kothari Date: Wed, 15 Sep 2021 11:03:59 +0000 Subject: [PATCH] Add support for additional tags Signed-off-by: GitHub --- api/openapi-spec/swagger.json | 7 +++++ docs/image.md | 1 + pkg/apis/build/v1alpha1/build_validation.go | 2 +- pkg/apis/build/v1alpha2/build_validation.go | 2 +- pkg/apis/build/v1alpha2/image_builds.go | 8 +++--- pkg/apis/build/v1alpha2/image_builds_test.go | 18 ++++++++++++- pkg/apis/build/v1alpha2/image_types.go | 2 ++ pkg/apis/build/v1alpha2/image_validation.go | 27 +++++++++++++++++++ .../build/v1alpha2/image_validation_test.go | 14 ++++++++++ .../build/v1alpha2/zz_generated.deepcopy.go | 5 ++++ pkg/apis/validate/validation_helpers.go | 4 +-- pkg/openapi/openapi_generated.go | 20 +++++++++++++- 12 files changed, 101 insertions(+), 9 deletions(-) diff --git a/api/openapi-spec/swagger.json b/api/openapi-spec/swagger.json index 95f6b39d0..13734bfa3 100644 --- a/api/openapi-spec/swagger.json +++ b/api/openapi-spec/swagger.json @@ -5832,6 +5832,13 @@ "source" ], "properties": { + "additionalTags": { + "type": "array", + "items": { + "type": "string" + }, + "x-kubernetes-list-type": "" + }, "build": { "$ref": "#/definitions/kpack.core.v1alpha1.ImageBuild" }, diff --git a/docs/image.md b/docs/image.md index 653c71ae8..9b53fbc3f 100644 --- a/docs/image.md +++ b/docs/image.md @@ -6,6 +6,7 @@ kpack will monitor the inputs to the image configuration to rebuild the image wh The following defines the relevant fields of the `image` resource spec in more detail: - `tag`: The image tag. +- `additionalTags`: Any additional list of image tags that should be published. This list of tags is mutable. - `builder`: Configuration of the `builder` resource the image builds will use. See more info [Builder Configuration](builders.md). - `serviceAccount`: The Service Account name that will be used for credential lookup. - `source`: The source code that will be monitored/built into images. See the [Source Configuration](#source-config) section below. diff --git a/pkg/apis/build/v1alpha1/build_validation.go b/pkg/apis/build/v1alpha1/build_validation.go index ba2792834..cbf8d4f0c 100644 --- a/pkg/apis/build/v1alpha1/build_validation.go +++ b/pkg/apis/build/v1alpha1/build_validation.go @@ -21,7 +21,7 @@ func (b *Build) Validate(ctx context.Context) *apis.FieldError { func (bs *BuildSpec) Validate(ctx context.Context) *apis.FieldError { return validate.ListNotEmpty(bs.Tags, "tags"). - Also(validate.Tags(bs.Tags)). + Also(validate.Tags(bs.Tags, "tags")). Also(bs.Builder.Validate(ctx).ViaField("builder")). Also(bs.Source.Validate(ctx).ViaField("source")). Also(bs.Bindings.Validate(ctx).ViaField("bindings")). diff --git a/pkg/apis/build/v1alpha2/build_validation.go b/pkg/apis/build/v1alpha2/build_validation.go index 1b4880427..f4eab52cd 100644 --- a/pkg/apis/build/v1alpha2/build_validation.go +++ b/pkg/apis/build/v1alpha2/build_validation.go @@ -21,7 +21,7 @@ func (b *Build) Validate(ctx context.Context) *apis.FieldError { func (bs *BuildSpec) Validate(ctx context.Context) *apis.FieldError { return validate.ListNotEmpty(bs.Tags, "tags"). - Also(validate.Tags(bs.Tags)). + Also(validate.Tags(bs.Tags, "tags")). Also(bs.Cache.Validate(ctx).ViaField("cache")). Also(bs.Builder.Validate(ctx).ViaField("builder")). Also(bs.Source.Validate(ctx).ViaField("source")). diff --git a/pkg/apis/build/v1alpha2/image_builds.go b/pkg/apis/build/v1alpha2/image_builds.go index 7a47a9caf..c2cb1b599 100644 --- a/pkg/apis/build/v1alpha2/image_builds.go +++ b/pkg/apis/build/v1alpha2/image_builds.go @@ -182,7 +182,7 @@ func (im *Image) SourceResolver() *SourceResolver { func (im *Image) generateTags(buildNumber string) []string { if im.disableAdditionalImageNames() { - return []string{im.Spec.Tag} + return append([]string{im.Spec.Tag}, im.Spec.AdditionalTags...) } now := time.Now() @@ -197,9 +197,11 @@ func (im *Image) generateTags(buildNumber string) []string { if tagName == "latest-" { tagName = "" } - return []string{ + return append([]string{ im.Spec.Tag, - tag.RegistryStr() + "/" + tag.RepositoryStr() + ":" + tagName + "b" + buildNumber + "." + now.Format("20060102") + "." + fmt.Sprintf("%02d%02d%02d", now.Hour(), now.Minute(), now.Second())} + tag.RegistryStr() + "/" + tag.RepositoryStr() + ":" + tagName + "b" + buildNumber + "." + now.Format("20060102") + "." + fmt.Sprintf("%02d%02d%02d", now.Hour(), now.Minute(), now.Second())}, + im.Spec.AdditionalTags..., + ) } func (im *Image) generateBuildName(buildNumber string) string { diff --git a/pkg/apis/build/v1alpha2/image_builds_test.go b/pkg/apis/build/v1alpha2/image_builds_test.go index 74ebd582c..3bd04c108 100644 --- a/pkg/apis/build/v1alpha2/image_builds_test.go +++ b/pkg/apis/build/v1alpha2/image_builds_test.go @@ -166,13 +166,21 @@ func testImageBuilds(t *testing.T, when spec.G, it spec.S) { assert.Equal(t, build.Spec.Source.Registry.Image, "some-registry.io/some-image") }) - it("with excludes additional tags names when explicitly disabled", func() { + it("with excludes additional autogenerated tags names when explicitly disabled", func() { image.Spec.Tag = "imagename/foo:test" image.Spec.ImageTaggingStrategy = corev1alpha1.None build := image.Build(sourceResolver, builder, latestBuild, "", "", 1) require.Len(t, build.Spec.Tags, 1) }) + it("with adds additional tags names", func() { + image.Spec.Tag = "imagename/foo:test" + image.Spec.AdditionalTags = []string{"imagename/foo:test2", "anotherimage/foo:test3"} + image.Spec.ImageTaggingStrategy = corev1alpha1.None + build := image.Build(sourceResolver, builder, latestBuild, "", "", 1) + require.Len(t, build.Spec.Tags, 3) + }) + it("generates a build with default process when set", func() { image.Spec.DefaultProcess = "sys-info" image.Name = "imageName" @@ -195,6 +203,14 @@ func testImageBuilds(t *testing.T, when spec.G, it spec.S) { require.Regexp(t, "gcr.io/imagename/notags:b1\\.\\d{8}\\.\\d{6}", build.Spec.Tags[1]) }) + it("with additional tags if provided", func() { + image.Spec.Tag = "gcr.io/imagename/tagged:latest" + image.Spec.AdditionalTags = []string{"imagename/foo:test2", "anotherimage/foo:test3"} + build := image.Build(sourceResolver, builder, latestBuild, "", "", 1) + require.Len(t, build.Spec.Tags, 4) + require.Regexp(t, "gcr.io/imagename/tagged:b1\\.\\d{8}\\.\\d{6}", build.Spec.Tags[1]) + }) + it("without tag prefix if image name has the tag 'latest' provided", func() { image.Spec.Tag = "gcr.io/imagename/tagged:latest" build := image.Build(sourceResolver, builder, latestBuild, "", "", 1) diff --git a/pkg/apis/build/v1alpha2/image_types.go b/pkg/apis/build/v1alpha2/image_types.go index 4ea2289e8..40f972c8e 100644 --- a/pkg/apis/build/v1alpha2/image_types.go +++ b/pkg/apis/build/v1alpha2/image_types.go @@ -52,6 +52,8 @@ type ImageSpec struct { Build *corev1alpha1.ImageBuild `json:"build,omitempty"` Notary *corev1alpha1.NotaryConfig `json:"notary,omitempty"` DefaultProcess string `json:"defaultProcess,omitempty"` + // +listType + AdditionalTags []string `json:"additionalTags,omitempty"` } // +k8s:openapi-gen=true diff --git a/pkg/apis/build/v1alpha2/image_validation.go b/pkg/apis/build/v1alpha2/image_validation.go index 8464ac6ca..995cd8ad0 100644 --- a/pkg/apis/build/v1alpha2/image_validation.go +++ b/pkg/apis/build/v1alpha2/image_validation.go @@ -10,6 +10,7 @@ import ( "k8s.io/apimachinery/pkg/util/validation" "knative.dev/pkg/apis" + "github.com/google/go-containerregistry/pkg/name" corev1alpha1 "github.com/pivotal/kpack/pkg/apis/core/v1alpha1" "github.com/pivotal/kpack/pkg/apis/validate" ) @@ -80,6 +81,7 @@ func (i *Image) validateName(imageName string) *apis.FieldError { func (is *ImageSpec) ValidateSpec(ctx context.Context) *apis.FieldError { return is.validateTag(ctx). + Also(is.validateAdditionalTags(ctx)). Also(validateBuilder(is.Builder).ViaField("builder")). Also(is.Source.Validate(ctx).ViaField("source")). Also(is.Build.Validate(ctx).ViaField("build")). @@ -97,6 +99,31 @@ func (is *ImageSpec) validateTag(ctx context.Context) *apis.FieldError { return validate.Tag(is.Tag) } +func (is *ImageSpec) validateAdditionalTags(ctx context.Context) *apis.FieldError { + return validate.Tags(is.AdditionalTags, "additionalTags").Also(is.validateSameRegistry()) +} + +func (is *ImageSpec) validateSameRegistry() *apis.FieldError { + tag, err := name.NewTag(is.Tag, name.WeakValidation) + // We only care about the non-nil error cases here as we validate + // the tag validity in other methods which should display appropriate errors. + if err == nil { + for _, t := range is.AdditionalTags { + addT, err := name.NewTag(t, name.WeakValidation) + if err == nil { + if addT.RegistryStr() != tag.RegistryStr() { + return &apis.FieldError{ + Message: "all additionalTags must have the same registry as tag", + Paths: []string{"additionalTags"}, + Details: fmt.Sprintf("expected registry: %s, got: %s", tag.RegistryStr(), addT.RegistryStr()), + } + } + } + } + } + return nil +} + func (is *ImageSpec) validateVolumeCache(ctx context.Context) *apis.FieldError { if is.Cache.Volume != nil && ctx.Value(HasDefaultStorageClass) == nil { return apis.ErrGeneric("spec.cache.volume.size cannot be set with no default StorageClass") diff --git a/pkg/apis/build/v1alpha2/image_validation_test.go b/pkg/apis/build/v1alpha2/image_validation_test.go index 0a6393c99..789578547 100644 --- a/pkg/apis/build/v1alpha2/image_validation_test.go +++ b/pkg/apis/build/v1alpha2/image_validation_test.go @@ -159,6 +159,20 @@ func testImageValidation(t *testing.T, when spec.G, it spec.S) { assertValidationError(image, ctx, apis.ErrInvalidValue(image.Spec.Tag, "tag").ViaField("spec")) }) + it("invalid additional image tags", func() { + image.Spec.AdditionalTags = []string{"valid/tag", "invalid/tag@sha256:thisisatag", "also/invalid@@"} + assertValidationError(image, + ctx, + apis.ErrInvalidArrayValue(image.Spec.AdditionalTags[1], "additionalTags", 1). + Also(apis.ErrInvalidArrayValue(image.Spec.AdditionalTags[2], "additionalTags", 2)). + ViaField("spec")) + }) + + it("tags from multiple registries", func() { + image.Spec.AdditionalTags = []string{"valid/tag", "gcr.io/valid/tag"} + assertValidationError(image, ctx, errors.New("all additionalTags must have the same registry as tag: spec.additionalTags\nexpected registry: index.docker.io, got: gcr.io")) + }) + it("tag does not contain fully qualified digest", func() { image.Spec.Tag = "some/app@sha256:72d10a33e3233657832967acffce652b729961da5247550ea58b2c2389cddc68" diff --git a/pkg/apis/build/v1alpha2/zz_generated.deepcopy.go b/pkg/apis/build/v1alpha2/zz_generated.deepcopy.go index a9026ff72..9a67425ac 100644 --- a/pkg/apis/build/v1alpha2/zz_generated.deepcopy.go +++ b/pkg/apis/build/v1alpha2/zz_generated.deepcopy.go @@ -991,6 +991,11 @@ func (in *ImageSpec) DeepCopyInto(out *ImageSpec) { *out = new(v1alpha1.NotaryConfig) (*in).DeepCopyInto(*out) } + if in.AdditionalTags != nil { + in, out := &in.AdditionalTags, &out.AdditionalTags + *out = make([]string, len(*in)) + copy(*out, *in) + } return } diff --git a/pkg/apis/validate/validation_helpers.go b/pkg/apis/validate/validation_helpers.go index 06cb70b4b..22c31ffb0 100644 --- a/pkg/apis/validate/validation_helpers.go +++ b/pkg/apis/validate/validation_helpers.go @@ -44,13 +44,13 @@ func Tag(value string) *apis.FieldError { return nil } -func Tags(tags []string) *apis.FieldError { +func Tags(tags []string, fieldName string) *apis.FieldError { var errors *apis.FieldError = nil for i, tag := range tags { _, err := name.NewTag(tag, name.WeakValidation) if err != nil { //noinspection GoNilness - errors = errors.Also(apis.ErrInvalidArrayValue(tag, "tags", i)) + errors = errors.Also(apis.ErrInvalidArrayValue(tag, fieldName, i)) } } return errors diff --git a/pkg/openapi/openapi_generated.go b/pkg/openapi/openapi_generated.go index 509f92661..500599f3c 100644 --- a/pkg/openapi/openapi_generated.go +++ b/pkg/openapi/openapi_generated.go @@ -3243,6 +3243,24 @@ func schema_pkg_apis_build_v1alpha2_ImageSpec(ref common.ReferenceCallback) comm Format: "", }, }, + "additionalTags": { + VendorExtensible: spec.VendorExtensible{ + Extensions: spec.Extensions{ + "x-kubernetes-list-type": "", + }, + }, + SchemaProps: spec.SchemaProps{ + Type: []string{"array"}, + Items: &spec.SchemaOrArray{ + Schema: &spec.Schema{ + SchemaProps: spec.SchemaProps{ + Type: []string{"string"}, + Format: "", + }, + }, + }, + }, + }, }, Required: []string{"tag", "source"}, }, @@ -3955,7 +3973,7 @@ func schema_pkg_apis_core_v1alpha1_Condition(ref common.ReferenceCallback) commo "lastTransitionTime": { SchemaProps: spec.SchemaProps{ Description: "LastTransitionTime is the last time the condition transitioned from one status to another. We use VolatileTime in place of metav1.Time to exclude this from creating equality.Semantic differences (all other things held constant).", - Type: []string{"string"}, Format: "", + Type: []string{"string"}, Format: "", }, }, "reason": {