Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
93 changes: 75 additions & 18 deletions cmd/ci-operator-prowgen/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"path/filepath"
"strings"

"github.com/ghodss/yaml"
"github.com/sirupsen/logrus"

prowconfig "sigs.k8s.io/prow/pkg/config"
Expand All @@ -20,13 +21,15 @@ import (
"github.com/openshift/ci-tools/pkg/prowgen"
"github.com/openshift/ci-tools/pkg/registry"
"github.com/openshift/ci-tools/pkg/util"
"github.com/openshift/ci-tools/pkg/util/gzip"
)

type options struct {
config.Options

fromDir string
fromReleaseRepo bool
fromFile string

toDir string
toReleaseRepo bool
Expand All @@ -44,6 +47,7 @@ func bindOptions(flag *flag.FlagSet) *options {

flag.StringVar(&opt.fromDir, "from-dir", "", "Path to a directory with a directory structure holding ci-operator configuration files for multiple components")
flag.BoolVar(&opt.fromReleaseRepo, "from-release-repo", false, "If set, it behaves like --from-dir=$GOPATH/src/github.com/openshift/release/ci-operator/config")
flag.StringVar(&opt.fromFile, "from-file", "", "Path to a single ci-operator configuration file (metadata is read from zz_generated_metadata in the file)")

flag.StringVar(&opt.toDir, "to-dir", "", "Path to a directory with a directory structure holding Prow job configuration files for multiple components")
flag.BoolVar(&opt.toReleaseRepo, "to-release-repo", false, "If set, it behaves like --to-dir=$GOPATH/src/github.com/openshift/release/ci-operator/jobs")
Expand Down Expand Up @@ -74,21 +78,27 @@ func (o *options) process() error {
}
}

if o.fromDir == "" {
return fmt.Errorf("ci-operator-prowgen needs exactly one of `--from-{dir,release-repo}` options")
if o.fromFile == "" && o.fromDir == "" {
return fmt.Errorf("ci-operator-prowgen needs exactly one of `--from-{dir,release-repo,file}` options")
}

if o.fromFile != "" && o.fromDir != "" {
return fmt.Errorf("ci-operator-prowgen accepts only one of `--from-{dir,release-repo}` and `--from-file` options")
}

if o.toDir == "" {
return fmt.Errorf("ci-operator-prowgen needs exactly one of `--to-{dir,release-repo}` options")
}

// TODO: deprecate --from-dir
o.ConfigDir = o.fromDir
if err := o.Options.Validate(); err != nil {
return fmt.Errorf("failed to validate config options: %w", err)
}
if err := o.Options.Complete(); err != nil {
return fmt.Errorf("failed to complete config options: %w", err)
if o.fromFile == "" {
// TODO: deprecate --from-dir
o.ConfigDir = o.fromDir
if err := o.Options.Validate(); err != nil {
return fmt.Errorf("failed to validate config options: %w", err)
}
if err := o.Options.Complete(); err != nil {
return fmt.Errorf("failed to complete config options: %w", err)
}
}
if o.registryPath != "" {
refs, chains, workflows, _, _, _, observers, err := load.Registry(o.registryPath, load.RegistryFlag(0))
Expand All @@ -100,6 +110,46 @@ func (o *options) process() error {
return nil
}

func (o *options) generateJobsFromFile() error {
logrus.Infof("Reading config from %s", o.fromFile)
data, err := gzip.ReadFileMaybeGZIP(o.fromFile)
if err != nil {
return fmt.Errorf("failed to read config file: %w", err)
}
var configSpec cioperatorapi.ReleaseBuildConfiguration
if err := yaml.Unmarshal(data, &configSpec); err != nil {
return fmt.Errorf("failed to unmarshal config: %w", err)
}
info := configSpec.Metadata
if info.Org == "" || info.Repo == "" || info.Branch == "" {
return fmt.Errorf("zz_generated_metadata in %s must specify org, repo, and branch", o.fromFile)
}
logrus.Infof("Loaded config for %s/%s@%s", info.Org, info.Repo, info.Branch)
if o.resolver != nil {
resolved, err := registry.ResolveConfig(o.resolver, configSpec)
if err != nil {
return fmt.Errorf("failed to resolve configuration: %w", err)
}
configSpec = resolved
}
configSpec.UnresolvedConfigPath = o.fromFile
generated, err := prowgen.GenerateJobs(&configSpec, &info)
if err != nil {
Comment on lines +135 to +137
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

--unresolved-config is populated with a host path, not a repo path.

On Line 135, UnresolvedConfigPath is set to o.fromFile directly. In --from-file usage this can be an absolute/local path, which is then embedded in generated jobs and won’t exist inside the job container.

Suggested fix
-	configSpec.UnresolvedConfigPath = o.fromFile
+	// ci-operator runs in the repo checkout; unresolved config path must be repo-relative.
+	configSpec.UnresolvedConfigPath = filepath.Base(o.fromFile)
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
configSpec.UnresolvedConfigPath = o.fromFile
generated, err := prowgen.GenerateJobs(&configSpec, &info)
if err != nil {
// ci-operator runs in the repo checkout; unresolved config path must be repo-relative.
configSpec.UnresolvedConfigPath = filepath.Base(o.fromFile)
generated, err := prowgen.GenerateJobs(&configSpec, &info)
if err != nil {
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@cmd/ci-operator-prowgen/main.go` around lines 135 - 137, The code currently
assigns a host filesystem path (o.fromFile) directly to
configSpec.UnresolvedConfigPath which embeds an invalid container path in
generated jobs; change the assignment to a repo/container-relative name by
stripping host directories (e.g., configSpec.UnresolvedConfigPath =
filepath.Base(o.fromFile) or otherwise compute a repo-relative path) and ensure
filepath import is added and empty/null cases handled before calling
prowgen.GenerateJobs(&configSpec, &info). This keeps the generated job path
valid inside the job container while still using the provided file name.

return err
}
orgRepo := fmt.Sprintf("%s/%s", info.Org, info.Repo)
logrus.Infof("Generated %d presubmits, %d postsubmits, %d periodics",
len(generated.PresubmitsStatic[orgRepo]),
len(generated.PostsubmitsStatic[orgRepo]),
len(generated.Periodics))
logrus.Infof("Writing jobs to %s/%s/%s", o.toDir, info.Org, info.Repo)
if err := jc.WriteBranchToDir(o.toDir, info.Org, info.Repo, generated, prowgen.Generator); err != nil {
return err
}
logrus.Info("Done")
return nil
}

// generateJobsToDir generates prow job configuration into the dir provided by
// consuming ci-operator configuration.
func (o *options) generateJobsToDir(subDir string) error {
Expand Down Expand Up @@ -194,15 +244,22 @@ func main() {
logrus.WithError(err).Fatal("Failed to process arguments")
}

args := flagSet.Args()
if len(args) == 0 {
args = append(args, "")
}
logger := logrus.WithFields(logrus.Fields{"target": opt.toDir, "source": opt.fromDir})
for _, subDir := range args {
logger = logger.WithFields(logrus.Fields{"subdir": subDir})
if err := opt.generateJobsToDir(subDir); err != nil {
logger.WithError(err).Fatal("Failed to generate jobs")
if opt.fromFile != "" {
logger := logrus.WithFields(logrus.Fields{"target": opt.toDir, "source": opt.fromFile})
if err := opt.generateJobsFromFile(); err != nil {
logger.WithError(err).Fatal("Failed to generate jobs from file")
}
} else {
args := flagSet.Args()
if len(args) == 0 {
args = append(args, "")
}
logger := logrus.WithFields(logrus.Fields{"target": opt.toDir, "source": opt.fromDir})
for _, subDir := range args {
logger = logger.WithFields(logrus.Fields{"subdir": subDir})
if err := opt.generateJobsToDir(subDir); err != nil {
logger.WithError(err).Fatal("Failed to generate jobs")
}
}
}
}
107 changes: 107 additions & 0 deletions cmd/in-repo-config-plugin/bootstrap.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,107 @@
package main

import (
"fmt"

corev1 "k8s.io/api/core/v1"
prowv1 "sigs.k8s.io/prow/pkg/apis/prowjobs/v1"
prowconfig "sigs.k8s.io/prow/pkg/config"

jc "github.com/openshift/ci-tools/pkg/jobconfig"
)

func generateBootstrapJobs(org, repo, branch, prowgenImage, checkconfigImage string) *prowconfig.JobConfig {
orgrepo := fmt.Sprintf("%s/%s", org, repo)
branchRegex := jc.ExactlyBranch(branch)

return &prowconfig.JobConfig{
PresubmitsStatic: map[string][]prowconfig.Presubmit{
orgrepo: {generateConfigCheckerPresubmit(org, repo, branch, branchRegex, checkconfigImage)},
},
PostsubmitsStatic: map[string][]prowconfig.Postsubmit{
orgrepo: {generateProwgenPostsubmit(org, repo, branch, branchRegex, prowgenImage)},
},
}
}

func generateConfigCheckerPresubmit(org, repo, branch, branchRegex, image string) prowconfig.Presubmit {
name := fmt.Sprintf("pull-ci-%s-%s-%s-ci-operator-config-check", org, repo, branch)
trueBool := true
repoPath := fmt.Sprintf("/home/prow/go/src/github.com/%s/%s", org, repo)

return prowconfig.Presubmit{
JobBase: prowconfig.JobBase{
Name: name,
Agent: string(prowv1.KubernetesAgent),
Labels: map[string]string{
jc.LabelGenerator: string(pluginGenerator),
},
Spec: &corev1.PodSpec{
Containers: []corev1.Container{{
Name: "checkconfig",
Image: image,
Command: []string{"ci-operator-checkconfig"},
Args: []string{
fmt.Sprintf("--config-dir=%s/%s", repoPath, ciOperatorDir),
},
}},
},
UtilityConfig: prowconfig.UtilityConfig{
Decorate: &trueBool,
},
},
Brancher: prowconfig.Brancher{
Branches: []string{branchRegex},
},
RegexpChangeMatcher: prowconfig.RegexpChangeMatcher{
RunIfChanged: `\.ci-operator/`,
},
AlwaysRun: false,
}
}

func generateProwgenPostsubmit(org, repo, branch, branchRegex, image string) prowconfig.Postsubmit {
name := fmt.Sprintf("branch-ci-%s-%s-%s-prowgen", org, repo, branch)
trueBool := true
repoPath := fmt.Sprintf("/home/prow/go/src/github.com/%s/%s", org, repo)

return prowconfig.Postsubmit{
JobBase: prowconfig.JobBase{
Name: name,
Agent: string(prowv1.KubernetesAgent),
Labels: map[string]string{
jc.LabelGenerator: string(pluginGenerator),
},
Spec: &corev1.PodSpec{
Containers: []corev1.Container{{
Name: "prowgen",
Image: image,
Command: []string{"ci-operator-prowgen"},
Args: []string{
fmt.Sprintf("--from-file=%s/.ci-operator.yaml", repoPath),
"--to-dir=/etc/jobs",
},
VolumeMounts: []corev1.VolumeMount{{
Name: "job-configs",
MountPath: "/etc/jobs",
}},
}},
Volumes: []corev1.Volume{{
Name: "job-configs",
VolumeSource: corev1.VolumeSource{
PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{
ClaimName: "job-configs-nfs",
},
},
}},
},
UtilityConfig: prowconfig.UtilityConfig{
Decorate: &trueBool,
},
MaxConcurrency: 1,
},
Brancher: prowconfig.Brancher{
Branches: []string{branchRegex},
},
}
}
96 changes: 96 additions & 0 deletions cmd/in-repo-config-plugin/bootstrap_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,96 @@
package main

import (
"testing"
)

func TestGenerateBootstrapJobs(t *testing.T) {
org := "openshift"
repo := "installer"
branch := "main"
prowgenImage := "quay.io/openshift/ci-operator-prowgen:latest"
checkconfigImage := "quay.io/openshift/ci-operator-checkconfig:latest"

jobConfig := generateBootstrapJobs(org, repo, branch, prowgenImage, checkconfigImage)

orgrepo := "openshift/installer"

presubmits := jobConfig.PresubmitsStatic[orgrepo]
if len(presubmits) != 1 {
t.Fatalf("expected 1 presubmit, got %d", len(presubmits))
}

pre := presubmits[0]
expectedPreName := "pull-ci-openshift-installer-main-ci-operator-config-check"
if pre.Name != expectedPreName {
t.Errorf("expected presubmit name %q, got %q", expectedPreName, pre.Name)
}
if len(pre.Branches) != 1 || pre.Branches[0] != "^main$" {
t.Errorf("expected branch regex ^main$, got %v", pre.Branches)
}
if pre.RunIfChanged != `\.ci-operator/` {
t.Errorf("expected run_if_changed %q, got %q", `\.ci-operator/`, pre.RunIfChanged)
}
if pre.AlwaysRun {
t.Error("expected always_run to be false")
}
if pre.Spec == nil || len(pre.Spec.Containers) != 1 {
t.Fatal("expected 1 container in presubmit spec")
}
if pre.Spec.Containers[0].Image != checkconfigImage {
t.Errorf("expected image %q, got %q", checkconfigImage, pre.Spec.Containers[0].Image)
}

postsubmits := jobConfig.PostsubmitsStatic[orgrepo]
if len(postsubmits) != 1 {
t.Fatalf("expected 1 postsubmit, got %d", len(postsubmits))
}

post := postsubmits[0]
expectedPostName := "branch-ci-openshift-installer-main-prowgen"
if post.Name != expectedPostName {
t.Errorf("expected postsubmit name %q, got %q", expectedPostName, post.Name)
}
if len(post.Branches) != 1 || post.Branches[0] != "^main$" {
t.Errorf("expected branch regex ^main$, got %v", post.Branches)
}
if post.MaxConcurrency != 1 {
t.Errorf("expected max_concurrency 1, got %d", post.MaxConcurrency)
}
if post.Spec == nil || len(post.Spec.Containers) != 1 {
t.Fatal("expected 1 container in postsubmit spec")
}
container := post.Spec.Containers[0]
if container.Image != prowgenImage {
t.Errorf("expected image %q, got %q", prowgenImage, container.Image)
}
if len(container.VolumeMounts) != 1 || container.VolumeMounts[0].MountPath != "/etc/jobs" {
t.Error("expected volume mount at /etc/jobs")
}
if len(post.Spec.Volumes) != 1 || post.Spec.Volumes[0].PersistentVolumeClaim == nil {
t.Error("expected PVC volume for job-configs")
}
if post.Spec.Volumes[0].PersistentVolumeClaim.ClaimName != "job-configs-nfs" {
t.Errorf("expected PVC claim name job-configs-nfs, got %q", post.Spec.Volumes[0].PersistentVolumeClaim.ClaimName)
}
}

func TestGenerateBootstrapJobsEscapesBranch(t *testing.T) {
jobConfig := generateBootstrapJobs("org", "repo", "release-4.15", "img", "img")
orgrepo := "org/repo"

pre := jobConfig.PresubmitsStatic[orgrepo][0]
if pre.Branches[0] != `^release-4\.15$` {
t.Errorf("expected escaped branch regex, got %q", pre.Branches[0])
}

post := jobConfig.PostsubmitsStatic[orgrepo][0]
if post.Branches[0] != `^release-4\.15$` {
t.Errorf("expected escaped branch regex, got %q", post.Branches[0])
}

expectedPreName := "pull-ci-org-repo-release-4.15-ci-operator-config-check"
if pre.Name != expectedPreName {
t.Errorf("expected %q, got %q", expectedPreName, pre.Name)
}
}
Loading