Do not set base sha for pull request images.

Set only tagger fields needed for image type.
Fix validation regex for commit to align with expected date format.
Return error from TagOption to signal problems with setting it.
Checking if pr or sha strings passed to TagOption is empty. Return error when trying to set empty string.

cmd/image-builder/main.go

@@ -638,9 +638,16 @@ func (l *StrList) List() []string {
 }
 
 func getTags(pr, sha string, templates []tags.Tag) ([]tags.Tag, error) {
+	var taggerOptions []tags.TagOption
+	if len(pr) > 0 {
+		taggerOptions = append(taggerOptions, tags.PRNumber(pr))
+	}
+	if len(sha) > 0 {
+		taggerOptions = append(taggerOptions, tags.CommitSHA(sha))
+	}
 
 	// build a tag from commit SHA
-	tagger, err := tags.NewTagger(templates, tags.CommitSHA(sha), tags.PRNumber(pr))
+	tagger, err := tags.NewTagger(templates, taggerOptions...)
 	if err != nil {
 		return nil, fmt.Errorf("get tagger: %w", err)
 	}
@@ -910,17 +917,15 @@ func getEnvs(o options, dockerfilePath string) (map[string]string, error) {
 }
 
 func parseTags(o options) ([]tags.Tag, error) {
-	var pr string
-	sha := o.gitState.BaseCommitSHA
-	if o.gitState.isPullRequest {
-		pr = fmt.Sprint(o.gitState.PullRequestNumber)
+	var (
+		pr  string
+		sha string
+	)
+	if !o.gitState.isPullRequest && o.gitState.BaseCommitSHA == "" {
+		sha = o.gitState.BaseCommitSHA
 	}
-
-	// TODO (dekiel):
-	//  when running for pr we should enforce a sha to be empty because the base branch commit is not relevant for tags generated on pr.
-	//  This variable should better be named to represent what sha it holds.
-	if sha == "" {
-		return nil, fmt.Errorf("sha still empty")
+	if o.gitState.isPullRequest && o.gitState.PullRequestNumber > 0 {
+		pr = fmt.Sprint(o.gitState.PullRequestNumber)
 	}
 
 	// read tags from base64 encoded string if provided

configs/kaniko-build-config.yaml

@@ -1,7 +1,7 @@
 default-commit-tag:
   name: "default_tag"
   value: "v{{ .Date }}-{{ .ShortSHA }}"
-  validation: "^(v[0-9]+\\.[0-9]+\\.[0-9]+-\\d{8}-[0-9a-f]{7})$"
+  validation: "^(v[0-9]{8}-[0-9a-f]{8})$"
 default-pr-tag:
   name: "default_tag"
   value: "PR-{{ .PRNumber }}"

pkg/tags/options.go

@@ -4,28 +4,32 @@ import (
 	"fmt"
 )
 
-type TagOption func(o *Tagger)
+type TagOption func(o *Tagger) error
 
 func DateFormat(format string) TagOption {
-	return func(t *Tagger) {
+	return func(t *Tagger) error {
 		t.Date = t.Time.Format(format)
+		return nil
 	}
 }
 
 func CommitSHA(sha string) TagOption {
-	return func(t *Tagger) {
+	return func(t *Tagger) error {
+		if len(sha) == 0 {
+			return fmt.Errorf("sha cannot be empty")
+		}
 		t.CommitSHA = sha
-		// TODO (dekiel): This should be logged as a warning, not considered as an error
-		// if t.CommitSHA == "" {
-		// 	return nil, errors.New("variable CommitSHA is empty")
-		// }
 		t.ShortSHA = fmt.Sprintf("%.8s", t.CommitSHA)
+		return nil
 	}
 }
 
 func PRNumber(pr string) TagOption {
-	return func(t *Tagger) {
+	return func(t *Tagger) error {
+		if len(pr) == 0 {
+			return fmt.Errorf("pr number cannot be empty")
+		}
 		t.PRNumber = pr
-		// TODO (dekiel): The empty string should be logged as a warning.
+		return nil
 	}
 }

pkg/tags/tag.go

@@ -4,16 +4,15 @@ import (
 	"fmt"
 	"regexp"
 	"strings"
-
-	"gopkg.in/yaml.v3"
 )
 
 // Tag store informations about single Tag
 type Tag struct {
 	// Name which identifies single Tag
 	Name string `yaml:"name" json:"name"`
 	// Value of the tag or template of it
-	Value      string `yaml:"value" json:"value"`
+	Value string `yaml:"value" json:"value"`
+	// Validation is a regex pattern to validate the tag value after it has been parsed
 	Validation string `yaml:"validation" json:"validation"`
 }
 
@@ -38,31 +37,3 @@ func NewTagFromString(val string) (Tag, error) {
 
 	return t, nil
 }
-
-// UnmarshalYAML provides custom logic for unmarshalling tag into struct
-// If not name is given it will be replaced by default_tag.
-// It ensures that both use cases are supported
-// TODO (dekiel): yaml config can provide a name always.
-//
-//	This custom unmarshaller is not needed.
-func (t *Tag) UnmarshalYAML(value *yaml.Node) error {
-	var tagTemplate string
-
-	if err := value.Decode(&tagTemplate); err == nil {
-		t.Name = "default_tag"
-		t.Value = tagTemplate
-		return nil
-	}
-
-	var tag map[string]string
-
-	err := value.Decode(&tag)
-	if err != nil {
-		return err
-	}
-
-	t.Name = tag["name"]
-	t.Value = tag["value"]
-
-	return nil
-}

pkg/tags/tags.go

@@ -18,22 +18,17 @@ type Tagger struct {
 	Date      string
 }
 
-// TODO (@Ressetkk): Evaluate if we really need to implement it in central way
-// func (tg *Tagger) AddFlags(fs *flag.FlagSet) {
-//	fs.Var(&tg.tags, "tag", "Go-template based tag")
-// }
-
 func (tg *Tagger) Env(key string) string {
 	return os.Getenv(key)
 }
 
 func (tg *Tagger) ParseTags() ([]Tag, error) {
 	var parsed []Tag
-	for _, t := range tg.tags {
-		if len(t.Name) == 0 || len(t.Value) == 0 {
-			return nil, fmt.Errorf("tag name or value is empty, tag name: %s, tag value: %s", t.Name, t.Value)
+	for _, tag := range tg.tags {
+		if len(tag.Name) == 0 || len(tag.Value) == 0 {
+			return nil, fmt.Errorf("tag name or value is empty, tag name: %s, tag value: %s", tag.Name, tag.Value)
 		}
-		tmpl, err := template.New("tag").Parse(t.Value)
+		tmpl, err := template.New("tag").Parse(tag.Value)
 		if err != nil {
 			return nil, err
 		}
@@ -42,10 +37,7 @@ func (tg *Tagger) ParseTags() ([]Tag, error) {
 		if err != nil {
 			return nil, err
 		}
-		tag := Tag{
-			Name:  t.Name,
-			Value: buf.String(),
-		}
+		tag.Value = buf.String()
 		err = tg.validateTag(tag)
 		if err != nil {
 			return nil, err
@@ -57,15 +49,15 @@ func (tg *Tagger) ParseTags() ([]Tag, error) {
 }
 
 func (tg *Tagger) validateTag(tag Tag) error {
-	if tag.Name == "default_tag" && tag.Validation == "" {
-		return fmt.Errorf("default_tag validation is empty")
+	if tag.Name == "default_tag" && len(tag.Validation) == 0 {
+		return fmt.Errorf("default_tag validation is empty, tag: %s", tag.Value)
 	}
 	if tag.Validation != "" {
 		// Verify PR default tag. Check if value starts with PR- and is followed by a number
 		re := regexp.MustCompile(tag.Validation)
 		match := re.FindAllString(tag.Value, -1)
 		if match == nil {
-			return fmt.Errorf("default_tag validation failed")
+			return fmt.Errorf("tag validation failed, tag: %s, validation: %s", tag.Value, tag.Validation)
 		}
 	}
 	return nil
@@ -79,14 +71,10 @@ func NewTagger(tags []Tag, opts ...TagOption) (*Tagger, error) {
 		Date: now.Format("20060102"),
 	}
 	for _, o := range opts {
-		o(&t)
+		err := o(&t)
+		if err != nil {
+			return nil, fmt.Errorf("error applying tag option: %w", err)
+		}
 	}
-	// TODO (dekiel): this should be valideted outside of constructor.
-	//  The tagger should be able to work for pr default tag or commit default tag.
-	//  The different default tags require different values.
-	// if t.CommitSHA == "" {
-	// 	return nil, errors.New("variable CommitSHA is empty")
-	// }
-	// t.ShortSHA = fmt.Sprintf("%.8s", t.CommitSHA)
 	return &t, nil
 }