Fixing unit tests

Makefile

@@ -38,16 +38,22 @@ generate-action:
  go run github.com/openshift-knative/hack/cmd/update-konflux-gen-action
 .PHONY: generate-action
 
+test: unit-tests
+.PHONY: test
+
 unit-tests:
- go test ./pkg/...
+ go run gotest.tools/gotestsum@latest \
+ --format testname -- \
+ -count=1 -race -timeout=10m \
+ ./pkg/...
 
  rm -rf openshift/project/testoutput
  rm -rf openshift/project/.github
 
  mkdir -p openshift
  go run ./cmd/update-konflux-gen-action --input ".github/workflows/release-generate-ci-template.yaml" --config "config/" --output "openshift/release-generate-ci.yaml"
  # If the following fails, please run 'make generate-action'
- diff -r "openshift/release-generate-ci.yaml" ".github/workflows/release-generate-ci.yaml"
+ diff -ur "openshift/release-generate-ci.yaml" ".github/workflows/release-generate-ci.yaml"
 
  go run ./cmd/generate/ --generators dockerfile \
  --project-file pkg/project/testdata/project.yaml \
@@ -57,7 +63,7 @@ unit-tests:
  --images-from "hack" \
  --images-from-url-format "https://raw.githubusercontent.com/openshift-knative/%s/%s/pkg/project/testdata/additional-images.yaml" \
  --output "openshift/project/testoutput/openshift"
- diff -r "pkg/project/testoutput" "openshift/project/testoutput"
+ diff -ur "pkg/project/testoutput" "openshift/project/testoutput"
 
 .PHONY: unit-tests
 

go.mod

@@ -7,6 +7,7 @@ replace k8s.io/client-go => k8s.io/client-go v0.27.2
 require (
  github.com/asottile/dockerfile v3.1.0+incompatible
  github.com/coreos/go-semver v0.3.0
+ github.com/distribution/reference v0.6.0
  github.com/ghodss/yaml v1.0.0
  github.com/google/go-cmp v0.6.0
  github.com/openshift/ci-tools v0.0.0-20240219083709-f792a49ba107

go.sum

@@ -144,6 +144,8 @@ github.com/dgrijalva/jwt-go v3.2.0+incompatible/go.mod h1:E3ru+11k8xSBh+hMPgOLZm
 github.com/dgrijalva/jwt-go/v4 v4.0.0-preview1 h1:CaO/zOnF8VvUfEbhRatPcwKVWamvbYd8tQGRWacE9kU=
 github.com/dgrijalva/jwt-go/v4 v4.0.0-preview1/go.mod h1:+hnT3ywWDTAFrW5aE+u2Sa/wT555ZqwoCS+pk3p6ry4=
 github.com/dimchansky/utfbom v1.1.0/go.mod h1:rO41eb7gLfo8SF1jd9F8HplJm1Fewwi4mQvIirEdv+8=
+github.com/distribution/reference v0.6.0 h1:0IXCQ5g4/QMHHkarYzh5l+u8T3t73zM5QvfrDyIgxBk=
+github.com/distribution/reference v0.6.0/go.mod h1:BbU0aIcezP1/5jX/8MP0YiH4SdvB5Y4f/wlDRiLyi3E=
 github.com/docker/spdystream v0.0.0-20160310174837-449fdfce4d96/go.mod h1:Qh8CwZgvJUkLughtfhJv5dyTYa91l1fOUCrgjqmcifM=
 github.com/docopt/docopt-go v0.0.0-20180111231733-ee0de3bc6815/go.mod h1:WwZ+bS3ebgob9U8Nd0kOddGdZWjyMGR8Wziv+TBNwSE=
 github.com/elazarl/goproxy v0.0.0-20180725130230-947c36da3153/go.mod h1:/Zj4wYkgs4iZTTu3o/KG3Itv/qCCa8VVMlb3i9OVuzc=

pkg/prowgen/prowgen_images_discovery.go

@@ -12,6 +12,7 @@ import (
  "strings"
 
  "github.com/asottile/dockerfile"
+ "github.com/distribution/reference"
  cioperatorapi "github.com/openshift/ci-tools/pkg/api"
  "k8s.io/apimachinery/pkg/util/sets"
  "k8s.io/utils/strings/slices"
@@ -22,12 +23,12 @@ const (
 )
 
 var (
- ciRegistryRegex = regexp.MustCompile(`registry\.(|svc\.)ci\.openshift\.org/\S+`)
+ ciRegistryRegex = regexp.MustCompile(`registry\.(|svc\.)ci\.openshift\.org`)
 
  ubiMinimal8Regex = regexp.MustCompile(`registry\.access\.redhat\.com/ubi8-minimal:latest$`)
  ubiMinimal9Regex = regexp.MustCompile(`registry\.access\.redhat\.com/ubi9-minimal:latest$`)
 
- imageOverrides = map[*regexp.Regexp]orgRepoTag{
+ imageOverrides = map[*regexp.Regexp]ociReference{
  ubiMinimal8Regex: {Org: "ocp", Repo: "ubi-minimal", Tag: "8"},
  ubiMinimal9Regex: {Org: "ocp", Repo: "ubi-minimal", Tag: "9"},
  }
@@ -44,13 +45,15 @@ var (
  }
 )
 
-type orgRepoTag struct {
- Org string
- Repo string
- Tag string
+type ociReference struct {
+ Domain string
+ Org string
+ Repo string
+ Tag string
+ Digest string
 }
 
-func (ort orgRepoTag) String() string {
+func (ort ociReference) String() string {
  return ort.Org + "_" + ort.Repo + "_" + ort.Tag
 }
 
@@ -191,32 +194,37 @@ func discoverInputImages(dockerfile string) (map[string]cioperatorapi.ImageStrea
  if imagePath == srcImage {
  inputImages[srcImage] = cioperatorapi.ImageBuildInputs{As: []string{srcImage}}
  } else {
- orgRepoTag, err := orgRepoTagFromPullString(imagePath)
+ ref, err := parseOciReference(imagePath)
  if err != nil {
  return nil, nil, fmt.Errorf("failed to parse string %s as pullspec: %w", imagePath, err)
  }
 
  for k, override := range imageOverrides {
  if k.FindString(imagePath) != "" {
- orgRepoTag = override
+ ref = override
  break
  }
  }
 
- inputs := inputImages[orgRepoTag.String()]
- inputs.As = sets.NewString(inputs.As...).Insert(imagePath).List() //different registries can resolve to the same orgRepoTag
+ inputs := inputImages[ref.String()]
+ inputs.As = sets.NewString(inputs.As...).Insert(imagePath).List() //different registries can resolve to the same ociReference
 
- if orgRepoTag.Org == "_" {
+ if ref.Org == "" && ref.Domain == "" {
  // consider image as a base image alias
  inputImages[imagePath] = inputs
- } else {
- requiredBaseImages[orgRepoTag.String()] = cioperatorapi.ImageStreamTagReference{
- Namespace: orgRepoTag.Org,
- Name: orgRepoTag.Repo,
- Tag: orgRepoTag.Tag,
- }
- inputImages[orgRepoTag.String()] = inputs
+ continue
+ }
+ if !ciRegistryRegex.MatchString(ref.Domain) {
+ // don't include images from other registries then registry.ci.openshift.org
+ continue
  }
+
+ requiredBaseImages[ref.String()] = cioperatorapi.ImageStreamTagReference{
+ Namespace: ref.Org,
+ Name: ref.Repo,
+ Tag: ref.Tag,
+ }
+ inputImages[ref.String()] = inputs
  }
  }
 
@@ -230,19 +238,28 @@ func getPullStringsFromDockerfile(filename string) ([]string, error) {
  }
 
  images := make([]string, 0, 1)
+ aliases := make(map[string]string)
  for _, cmd := range cmds {
  if cmd.Cmd == "FROM" {
- if len(cmd.Value) != 1 {
- return nil, fmt.Errorf("expected one value for FROM "+
- "command, got %d: %q", len(cmd.Value), cmd.Value)
+ if len(cmd.Value) == 1 {
+ images = append(images, cmd.Value[0])
+ continue
+ }
+ if len(cmd.Value) == 3 && strings.ToLower(cmd.Value[1]) == "as" {
+ aliases[cmd.Value[2]] = cmd.Value[0]
+ images = append(images, cmd.Value[0])
+ continue
  }
- images = append(images, cmd.Value[0])
- continue
+ return nil, fmt.Errorf("invalid FROM stanza: %q", cmd.Value)
  }
  if cmd.Cmd == "COPY" || cmd.Cmd == "ADD" {
  for _, fl := range cmd.Flags {
  if strings.HasPrefix(fl, "--from=") {
- images = append(images, strings.TrimPrefix(fl, "--from="))
+ imageOrAlias := strings.TrimPrefix(fl, "--from=")
+ if _, ok := aliases[imageOrAlias]; !ok {
+ // not an alias, consider it as an image
+ images = append(images, imageOrAlias)
+ }
  }
  }
  }
@@ -274,28 +291,31 @@ func isSourceOrBuildDockerfile(dockerfile string) bool {
  strings.Contains(dockerfile, "build-image")
 }
 
-func orgRepoTagFromPullString(pullString string) (orgRepoTag, error) {
- res := orgRepoTag{Tag: "latest"}
- slashSplit := strings.Split(pullString, "/")
- switch n := len(slashSplit); n {
- case 1:
- res.Org = "_"
- res.Repo = slashSplit[0]
- case 2:
- res.Org = slashSplit[0]
- res.Repo = slashSplit[1]
- case 3:
- res.Org = slashSplit[1]
- res.Repo = slashSplit[2]
- default:
- return res, fmt.Errorf("pull string %q couldn't be parsed, expected to get between one and three elements after slashsplitting, got %d", pullString, n)
+func parseOciReference(pullString string) (ociReference, error) {
+ ref, err := reference.Parse(pullString)
+ if err != nil {
+ return ociReference{}, fmt.Errorf("failed to parse %q as a reference: %w", pullString, err)
  }
- if repoTag := strings.Split(res.Repo, ":"); len(repoTag) == 2 {
- res.Repo = repoTag[0]
- res.Tag = repoTag[1]
+ res := ociReference{Tag: "latest"}
+ if named, ok := ref.(reference.Named); ok {
+ res.Domain = reference.Domain(named)
+ path := strings.SplitN(reference.Path(named), "/", 2)
+ if len(path) == 1 {
+ res.Repo = path[0]
+ } else {
+ res.Org = path[0]
+ res.Repo = path[1]
+ }
+ if tagged, ok := ref.(reference.Tagged); ok {
+ res.Tag = tagged.Tag()
+ }
+ if digested, ok := ref.(reference.Digested); ok {
+ res.Digest = digested.Digest().String()
+ }
+ return res, nil
  }
 
- return res, nil
+ return res, fmt.Errorf("pull string %q couldn't be parsed as OCI image", pullString)
 }
 
 func ToRegexp(s []string) []*regexp.Regexp {

pkg/prowgen/prowgen_tests.go

@@ -100,70 +100,76 @@ func DiscoverTests(r Repository, openShift OpenShift, sourceImageName string, sk
  AllowBestEffortPostSteps: pointer.Bool(true),
  AllowSkipOnSuccess: pointer.Bool(true),
  Environment: env,
- Test: []cioperatorapi.TestStep{{
- LiteralTestStep: &cioperatorapi.LiteralTestStep{
- As: "test",
- From: sourceImageName,
- Commands: test.EffectiveCommand(),
- Resources: cioperatorapi.ResourceRequirements{
- Requests: cioperatorapi.ResourceList{
- "cpu": "100m",
+ Test: []cioperatorapi.TestStep{
+ {
+ LiteralTestStep: &cioperatorapi.LiteralTestStep{
+ As: "test",
+ From: sourceImageName,
+ Commands: test.EffectiveCommand(),
+ Resources: cioperatorapi.ResourceRequirements{
+ Requests: cioperatorapi.ResourceList{
+ "cpu": "100m",
+ },
  },
+ Environment: test.EnvironmentAsStepParams(),
+ Timeout: testTimeout,
+ Dependencies: dependenciesFromImages(cfg.Images, test.SkipImages),
+ Cli: "latest",
  },
- Environment: test.EnvironmentAsStepParams(),
- Timeout: testTimeout,
- Dependencies: dependenciesFromImages(cfg.Images, test.SkipImages),
- Cli: "latest",
  },
- }},
- Post: []cioperatorapi.TestStep{{
- LiteralTestStep: &cioperatorapi.LiteralTestStep{
- As: "knative-must-gather",
- From: sourceImageName,
- Commands: `oc adm must-gather --image=quay.io/openshift-knative/must-gather --dest-dir "${ARTIFACT_DIR}/gather-knative"`,
- Resources: cioperatorapi.ResourceRequirements{
- Requests: cioperatorapi.ResourceList{
- "cpu": "100m",
+ },
+ Post: []cioperatorapi.TestStep{
+ {
+ LiteralTestStep: &cioperatorapi.LiteralTestStep{
+ As: "knative-must-gather",
+ From: sourceImageName,
+ Commands: `oc adm must-gather --image=quay.io/openshift-knative/must-gather --dest-dir "${ARTIFACT_DIR}/gather-knative"`,
+ Resources: cioperatorapi.ResourceRequirements{
+ Requests: cioperatorapi.ResourceList{
+ "cpu": "100m",
+ },
  },
+ Timeout: &prowapi.Duration{Duration: 20 * time.Minute},
+ BestEffort: pointer.Bool(true),
+ OptionalOnSuccess: pointer.Bool(true),
+ Cli: "latest",
  },
- Timeout: &prowapi.Duration{Duration: 20 * time.Minute},
- BestEffort: pointer.Bool(true),
- OptionalOnSuccess: pointer.Bool(true),
- Cli: "latest",
  },
- }, {
- LiteralTestStep: &cioperatorapi.LiteralTestStep{
- As: "openshift-must-gather",
- From: sourceImageName,
- Commands: `oc adm must-gather --dest-dir "${ARTIFACT_DIR}/gather-openshift"`,
- Resources: cioperatorapi.ResourceRequirements{
- Requests: cioperatorapi.ResourceList{
- "cpu": "100m",
+ {
+ LiteralTestStep: &cioperatorapi.LiteralTestStep{
+ As: "openshift-must-gather",
+ From: sourceImageName,
+ Commands: `oc adm must-gather --dest-dir "${ARTIFACT_DIR}/gather-openshift"`,
+ Resources: cioperatorapi.ResourceRequirements{
+ Requests: cioperatorapi.ResourceList{
+ "cpu": "100m",
+ },
  },
+ Timeout: &prowapi.Duration{Duration: 20 * time.Minute},
+ BestEffort: pointer.Bool(true),
+ OptionalOnSuccess: pointer.Bool(true),
+ Cli: "latest",
  },
- Timeout: &prowapi.Duration{Duration: 20 * time.Minute},
- BestEffort: pointer.Bool(true),
- OptionalOnSuccess: pointer.Bool(true),
- Cli: "latest",
  },
- }, {
- LiteralTestStep: &cioperatorapi.LiteralTestStep{
- As: "openshift-gather-extra",
- From: sourceImageName,
- Commands: `curl -skSL https://raw.githubusercontent.com/openshift/release/master/ci-operator/step-registry/gather/extra/gather-extra-commands.sh | /bin/bash -s`,
- GracePeriod: &prowapi.Duration{Duration: 60 * time.Second},
- Resources: cioperatorapi.ResourceRequirements{
- Requests: cioperatorapi.ResourceList{
- "cpu": "300m",
- "memory": "300Mi",
+ {
+ LiteralTestStep: &cioperatorapi.LiteralTestStep{
+ As: "openshift-gather-extra",
+ From: sourceImageName,
+ Commands: `curl -skSL https://raw.githubusercontent.com/openshift/release/master/ci-operator/step-registry/gather/extra/gather-extra-commands.sh | /bin/bash -s`,
+ GracePeriod: &prowapi.Duration{Duration: 60 * time.Second},
+ Resources: cioperatorapi.ResourceRequirements{
+ Requests: cioperatorapi.ResourceList{
+ "cpu": "300m",
+ "memory": "300Mi",
+ },
  },
+ Timeout: &prowapi.Duration{Duration: 20 * time.Minute},
+ BestEffort: pointer.Bool(true),
+ OptionalOnSuccess: pointer.Bool(true),
+ Cli: "latest",
  },
- Timeout: &prowapi.Duration{Duration: 20 * time.Minute},
- BestEffort: pointer.Bool(true),
- OptionalOnSuccess: pointer.Bool(true),
- Cli: "latest",
  },
- }},
+ },
  Workflow: workflow,
  },
  }
@@ -261,6 +267,9 @@ func (t *Test) EffectiveCommand() string {
 }
 
 func (t *Test) EnvironmentAsStepParams() []cioperatorapi.StepParameter {
+ if len(t.Environment) == 0 {
+ return nil
+ }
  params := make([]cioperatorapi.StepParameter, 0, len(t.Environment))
  for k, v := range t.Environment {
  params = append(params, cioperatorapi.StepParameter{