Skip to content
Merged
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
17 changes: 17 additions & 0 deletions api/v1alpha1/imagebuild_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,13 @@ type ImageBuildSpec struct {
// OperatorConfig at request time to prevent TOCTOU races.
// +optional
TaskBundleRef string `json:"taskBundleRef,omitempty"`

// TTL is the time-to-live for this build. The build is automatically deleted
// after this duration past its completion (or creation if still in progress).
// Uses Go duration format (e.g. "24h", "72h", "168h").
// Empty uses the OperatorConfig default. Set to "0" to disable expiry.
// +optional
TTL string `json:"ttl,omitempty"`
Comment on lines +97 to +102

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion | 🟠 Major

Reminder: regenerate CRDs/DeepCopy after type changes.

The PR checklist shows make manifests generate is unchecked. Adding Spec.TTL and Status.ExpiresAt requires regenerating both the CRD manifests (for validation, additionalPrinterColumns, etc.) and zz_generated.deepcopy.go so ImageBuildStatus.ExpiresAt (pointer type) is correctly deep-copied. Please ensure make generate manifests is run and committed.

As per coding guidelines: "After modifying types in api/v1alpha1/, run make generate manifests to regenerate DeepCopy methods and CRDs".

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@api/v1alpha1/imagebuild_types.go` around lines 80 - 85, You added a new field
Spec.TTL and Status.ExpiresAt but did not regenerate code/CRDs; run the
generator to update deepcopy and CRD manifests: execute "make generate
manifests" (or the project equivalent) to regenerate zz_generated.deepcopy.go
and the CRD YAMLs so ImageBuildStatus.ExpiresAt (pointer) is deep-copied and the
CRD validation/printer columns include Spec.TTL, then commit the generated
files.

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

# Check the TTL-related code in the controller
fd 'controller.go' api/v1alpha1 internal/controller

Repository: centos-automotive-suite/automotive-dev-operator

Length of output: 423


🏁 Script executed:

# Search for DefaultBuildTTL and DefaultBuildTimeoutMinutes definitions
rg 'DefaultBuildTTL|DefaultBuildTimeoutMinutes' --type go

Repository: centos-automotive-suite/automotive-dev-operator

Length of output: 1471


🏁 Script executed:

# Look at the controller.go file around lines 425-430
rg -n 'TTL' internal/controller --type go -B 3 -A 3

Repository: centos-automotive-suite/automotive-dev-operator

Length of output: 3777


🏁 Script executed:

# Check the imagebuild_types.go file for the TTL field
fd 'imagebuild_types.go' -type f

Repository: centos-automotive-suite/automotive-dev-operator

Length of output: 261


🏁 Script executed:

# Read the controller.go file to see the complete checkExpiry implementation
cat -n internal/controller/imagebuild/controller.go | sed -n '391,454p'

Repository: centos-automotive-suite/automotive-dev-operator

Length of output: 2551


🏁 Script executed:

# Also check around the lines mentioned (425-430)
cat -n internal/controller/imagebuild/controller.go | sed -n '415,440p'

Repository: centos-automotive-suite/automotive-dev-operator

Length of output: 957


🏁 Script executed:

# Look at ImageBuild status phases
rg -A 20 'type ImageBuildStatus' api/v1alpha1/imagebuild_types.go

Repository: centos-automotive-suite/automotive-dev-operator

Length of output: 910


🏁 Script executed:

# Search for test files related to TTL logic
find . -name '*test*.go' -type f | xargs grep -l 'TTL\|checkExpiry' 2>/dev/null

Repository: centos-automotive-suite/automotive-dev-operator

Length of output: 129


🏁 Script executed:

# Look for any phase-related logic in the reconciler
rg 'Phase|Status' internal/controller/imagebuild/controller.go | head -20

Repository: centos-automotive-suite/automotive-dev-operator

Length of output: 1223


🏁 Script executed:

# Check the current doc comment in imagebuild_types.go more carefully
cat -n api/v1alpha1/imagebuild_types.go | sed -n '75,90p'

Repository: centos-automotive-suite/automotive-dev-operator

Length of output: 961


🏁 Script executed:

# Look for any NoExpireAnnotation usage or documentation
rg 'NoExpireAnnotation|no.*expire|disable.*expiry' --type go -B 2 -A 2

Repository: centos-automotive-suite/automotive-dev-operator

Length of output: 8935


🏁 Script executed:

# Check if there are any integration tests or examples for TTL behavior
find . -name '*.yaml' -o -name '*.yml' | xargs grep -l 'ttl' 2>/dev/null | head -5

Repository: centos-automotive-suite/automotive-dev-operator

Length of output: 220


🏁 Script executed:

# Check the DefaultBuildTTL comment in operatorconfig_types.go to see if intent is documented
cat -n api/v1alpha1/operatorconfig_types.go | grep -A 5 'DefaultBuildTTL ='

Repository: centos-automotive-suite/automotive-dev-operator

Length of output: 396


🏁 Script executed:

# Check if there are any tests that verify TTL behavior with different phases
rg -B 5 -A 10 'checkExpiry|TTL.*in.*progress|in.*progress.*TTL' internal/controller/imagebuild --type go

Repository: centos-automotive-suite/automotive-dev-operator

Length of output: 2729


🏁 Script executed:

# Look at the reconcile loop to see when checkExpiry is called (in what phase)
rg -B 10 'checkExpiry' internal/controller/imagebuild/controller.go | head -40

Repository: centos-automotive-suite/automotive-dev-operator

Length of output: 1013


🏁 Script executed:

# Check the full DefaultBuildTTL comment for context
cat -n api/v1alpha1/operatorconfig_types.go | sed -n '60,75p'

Repository: centos-automotive-suite/automotive-dev-operator

Length of output: 909


TTL can delete builds in any phase, including in-progress ones — clarify intent and update documentation.

The code allows deletion of builds in Pending/Uploading/Building/Pushing/Flashing phases if their CreationTimestamp + TTL elapses (since CompletionTime is nil). The constant comment in api/v1alpha1/operatorconfig_types.go (line 63) states "for completed/failed builds," but the actual checkExpiry logic (lines 425-430 of internal/controller/imagebuild/controller.go) applies TTL deletion to any phase without filtering. This creates a documentation-code mismatch.

Either confirm this behavior is intentional and update the doc comments accordingly, or restrict expiry to terminal phases (Completed/Failed) only. If intentional, also note that user-set short TTLs combined with long artifact uploads could trigger mid-flight deletion despite the 24h default.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@api/v1alpha1/imagebuild_types.go` around lines 80 - 85, The TTL behavior is
inconsistent with its documentation: either make expiry only apply to terminal
builds or document that TTL can delete in-flight builds; to fix, decide which
behavior you want and implement one of two changes — (A) restrict the expiry
check in checkExpiry (internal/controller/imagebuild/controller.go) to only
consider builds with CompletionTime set or Phase == "Completed"/"Failed" (so
in-progress phases are excluded), or (B) if mid-flight expiry is intended,
update the TTL field comment in ImageBuild (api/v1alpha1/imagebuild_types.go)
and the related OperatorConfig comment (api/v1alpha1/operatorconfig_types.go) to
state TTL applies to any phase and warn about short user-set TTLs interacting
with long uploads; ensure the updated docs mention the "0" disable sentinel and
the potential for in-progress deletion.

}

// FlashSpec defines configuration for flashing images to hardware via Jumpstarter
Expand Down Expand Up @@ -256,6 +263,11 @@ type ImageBuildStatus struct {
// LeaseID is the Jumpstarter lease ID acquired during flash
// +optional
LeaseID string `json:"leaseId,omitempty"`

// ExpiresAt is when this build will be automatically deleted.
// Nil if expiry is disabled (TTL "0" or no-expire annotation).
// +optional
ExpiresAt *metav1.Time `json:"expiresAt,omitempty"`
}

// +kubebuilder:object:root=true
Expand Down Expand Up @@ -511,3 +523,8 @@ func (s *ImageBuildSpec) GetFlashLeaseName() string {
}
return ""
}

// GetTTL returns the per-build TTL string, or empty if not set
func (s *ImageBuildSpec) GetTTL() string {
return s.TTL
}
35 changes: 35 additions & 0 deletions api/v1alpha1/operatorconfig_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,12 @@ const (
// DefaultAutoPauseTimeoutMinutes is the default idle timeout in minutes before a workspace is auto-paused
DefaultAutoPauseTimeoutMinutes int32 = 30

// DefaultBuildTTL is the default time-to-live for builds (all phases, including in-progress).
DefaultBuildTTL = "24h"

// NoExpireAnnotation prevents automatic expiry when set to "true" on an ImageBuild
NoExpireAnnotation = "automotive.sdv.cloud.redhat.com/no-expire"

// BuildServiceAccountName is the dedicated SA used by build pods and token minting.
// Using a dedicated SA avoids collisions with the shared "pipeline" SA.
BuildServiceAccountName = "ado-build"
Expand Down Expand Up @@ -543,6 +549,19 @@ type OSBuildsConfig struct {
// Example: "quay.io/rh-sdv-cloud/automotive-dev-tekton-tasks:v0.1.0@sha256:abc123..."
// +optional
TaskBundleRef string `json:"taskBundleRef,omitempty"`

// DefaultBuildTTL is the default time-to-live for builds (all phases, including in-progress).
// Builds are automatically deleted after this duration. Uses Go duration format (e.g. "24h", "72h").
// Set to "0" to disable expiry by default.
// Default: "24h"
// +optional
DefaultBuildTTL string `json:"defaultBuildTTL,omitempty"`

// MaxBuildTTL is the maximum TTL that users can request for individual builds.
// Build requests specifying a TTL greater than this value are rejected.
// Set to "0" for no maximum. Uses Go duration format (e.g. "168h" for 1 week).
// +optional
MaxBuildTTL string `json:"maxBuildTTL,omitempty"`
Comment thread
coderabbitai[bot] marked this conversation as resolved.
}

// CertificateSourceRef references a Secret or ConfigMap that contains trusted CA certificates.
Expand Down Expand Up @@ -578,6 +597,22 @@ func (c *OSBuildsConfig) GetFlashTimeoutMinutes() int32 {
return DefaultFlashTimeoutMinutes
}

// GetDefaultBuildTTL returns the default build TTL string, falling back to the hardcoded default
func (c *OSBuildsConfig) GetDefaultBuildTTL() string {
if c != nil && c.DefaultBuildTTL != "" {
return c.DefaultBuildTTL
}
return DefaultBuildTTL
}

// GetMaxBuildTTL returns the max build TTL string, or empty if no max is set
func (c *OSBuildsConfig) GetMaxBuildTTL() string {
if c != nil {
return c.MaxBuildTTL
}
return ""
}

// GetUsePVCScratchVolumes returns whether to use PVC-backed scratch volumes (default: true)
func (c *OSBuildsConfig) GetUsePVCScratchVolumes() bool {
if c.UsePVCScratchVolumes != nil {
Expand Down
4 changes: 4 additions & 0 deletions api/v1alpha1/zz_generated.deepcopy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 4 additions & 0 deletions cmd/caib/buildcmd/build.go
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ type Options struct {
InternalRegistryTag *string

SecureBuild *bool
TTL *string

InsecureSkipTLS *bool

Expand Down Expand Up @@ -451,6 +452,7 @@ func (h *Handler) RunBuild(cmd *cobra.Command, args []string) {
BuilderImage: *h.opts.BuilderImage,
RebuildBuilder: *h.opts.RebuildBuilder,
SecureBuild: *h.opts.SecureBuild,
TTL: *h.opts.TTL,
}

if err := h.applyRegistryCredentialsToRequest(&req); err != nil {
Expand Down Expand Up @@ -563,6 +565,7 @@ func (h *Handler) RunDisk(cmd *cobra.Command, args []string) {
Compression: *h.opts.CompressionAlgo,
ExportOCI: *h.opts.ExportOCI,
SecureBuild: *h.opts.SecureBuild,
TTL: *h.opts.TTL,
}

if err := h.applyRegistryCredentialsToRequest(&req); err != nil {
Expand Down Expand Up @@ -692,6 +695,7 @@ func (h *Handler) RunBuildDev(cmd *cobra.Command, args []string) {
Compression: *h.opts.CompressionAlgo,
ExportOCI: *h.opts.ExportOCI,
SecureBuild: *h.opts.SecureBuild,
TTL: *h.opts.TTL,
}

if err := h.applyRegistryCredentialsToRequest(&req); err != nil {
Expand Down
2 changes: 2 additions & 0 deletions cmd/caib/buildcmd/build_disk_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ func newTestDiskOpts() Options {
internalRegImageName string
internalRegTag string
secureBuild bool
buildTTL string
insecureSkipTLS bool
)
return Options{
Expand Down Expand Up @@ -83,6 +84,7 @@ func newTestDiskOpts() Options {
InternalRegistryImageName: &internalRegImageName,
InternalRegistryTag: &internalRegTag,
SecureBuild: &secureBuild,
TTL: &buildTTL,
InsecureSkipTLS: &insecureSkipTLS,
}
}
Expand Down
4 changes: 4 additions & 0 deletions cmd/caib/image/image.go
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ type Options struct {
InternalRegistryTag *string

SecureBuild *bool
TTL *string

SealedBuilderImage *string
SealedArchitecture *string
Expand Down Expand Up @@ -160,6 +161,7 @@ func NewImageCmd(opts Options) *cobra.Command {
buildCmd.Flags().StringVar(opts.ExporterSelector, "exporter", "", "direct exporter selector for flash (alternative to --target lookup)")
// Secure build
buildCmd.Flags().BoolVar(opts.SecureBuild, "secure", false, "resolve tasks from signed Tekton Bundle (requires OperatorConfig taskBundleRef)")
buildCmd.Flags().StringVar(opts.TTL, "ttl", "", "time-to-live for the build (e.g. 24h, 72h, 168h); empty=server default, 0=no expiry")
// Internal registry options
buildCmd.Flags().BoolVar(opts.UseInternalRegistry, "internal-registry", false, "push to OpenShift internal registry")
buildCmd.Flags().StringVar(opts.InternalRegistryImageName, "image-name", "", "override image name for internal registry (default: build name)")
Expand Down Expand Up @@ -217,6 +219,7 @@ func NewImageCmd(opts Options) *cobra.Command {
diskCmd.Flags().StringVar(opts.ExporterSelector, "exporter", "", "direct exporter selector for flash (alternative to --target lookup)")
// Secure build
diskCmd.Flags().BoolVar(opts.SecureBuild, "secure", false, "resolve tasks from signed Tekton Bundle (requires OperatorConfig taskBundleRef)")
diskCmd.Flags().StringVar(opts.TTL, "ttl", "", "time-to-live for the build (e.g. 24h, 72h, 168h); empty=server default, 0=no expiry")
// Internal registry options
diskCmd.Flags().BoolVar(opts.UseInternalRegistry, "internal-registry", false, "push to OpenShift internal registry")
diskCmd.Flags().StringVar(opts.InternalRegistryImageName, "image-name", "", "override image name for internal registry (default: build name)")
Expand Down Expand Up @@ -261,6 +264,7 @@ func NewImageCmd(opts Options) *cobra.Command {
buildDevCmd.Flags().StringVar(opts.ExporterSelector, "exporter", "", "direct exporter selector for flash (alternative to --target lookup)")
// Secure build
buildDevCmd.Flags().BoolVar(opts.SecureBuild, "secure", false, "resolve tasks from signed Tekton Bundle (requires OperatorConfig taskBundleRef)")
buildDevCmd.Flags().StringVar(opts.TTL, "ttl", "", "time-to-live for the build (e.g. 24h, 72h, 168h); empty=server default, 0=no expiry")
// Internal registry options
buildDevCmd.Flags().BoolVar(opts.UseInternalRegistry, "internal-registry", false, "push to OpenShift internal registry")
buildDevCmd.Flags().StringVar(opts.InternalRegistryImageName, "image-name", "", "override image name for internal registry (default: build name)")
Expand Down
3 changes: 3 additions & 0 deletions cmd/caib/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,9 @@ var (
// Secure build
secureBuild bool

// Build TTL
buildTTL string

// TLS options
insecureSkipTLS bool

Expand Down
4 changes: 4 additions & 0 deletions cmd/caib/runtime_wiring.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ type runtimeState struct {
InternalRegistryTag *string

SecureBuild *bool
TTL *string

InsecureSkipTLS *bool

Expand Down Expand Up @@ -115,6 +116,7 @@ func newRuntimeState() runtimeState {
InternalRegistryTag: &internalRegistryTag,

SecureBuild: &secureBuild,
TTL: &buildTTL,

InsecureSkipTLS: &insecureSkipTLS,

Expand Down Expand Up @@ -180,6 +182,7 @@ func (s runtimeState) newHandlers() handlerSet {
InternalRegistryImageName: s.InternalRegistryImageName,
InternalRegistryTag: s.InternalRegistryTag,
SecureBuild: s.SecureBuild,
TTL: s.TTL,
InsecureSkipTLS: s.InsecureSkipTLS,
HandleError: handleError,
}),
Expand Down Expand Up @@ -302,6 +305,7 @@ func (s runtimeState) imageOptions(h handlerSet) image.Options {
InternalRegistryTag: s.InternalRegistryTag,

SecureBuild: s.SecureBuild,
TTL: s.TTL,

SealedBuilderImage: s.SealedBuilderImage,
SealedArchitecture: s.SealedArchitecture,
Expand Down
13 changes: 13 additions & 0 deletions config/crd/bases/automotive.sdv.cloud.redhat.com_imagebuilds.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -215,6 +215,13 @@ spec:
used for this build. Set automatically by the Build API from the
OperatorConfig at request time to prevent TOCTOU races.
type: string
ttl:
description: |-
TTL is the time-to-live for this build. The build is automatically deleted
after this duration past its completion (or creation if still in progress).
Uses Go duration format (e.g. "24h", "72h", "168h").
Empty uses the OperatorConfig default. Set to "0" to disable expiry.
type: string
workspace:
description: |-
Workspace is the name of the Workspace CR this build belongs to.
Expand Down Expand Up @@ -296,6 +303,12 @@ spec:
- type
type: object
type: array
expiresAt:
description: |-
ExpiresAt is when this build will be automatically deleted.
Nil if expiry is disabled (TTL "0" or no-expire annotation).
format: date-time
type: string
flashTaskRunName:
description: FlashTaskRunName is the name of the TaskRun for flashing
to hardware
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -722,6 +722,13 @@ spec:
Required for bootc builds to allow nested containers to pull builder images
Example: "default-route-openshift-image-registry.apps.mycluster.example.com"
type: string
defaultBuildTTL:
description: |-
DefaultBuildTTL is the default time-to-live for builds (all phases, including in-progress).
Builds are automatically deleted after this duration. Uses Go duration format (e.g. "24h", "72h").
Set to "0" to disable expiry by default.
Default: "24h"
type: string
enabled:
default: true
description: Enabled determines if Tekton tasks for OS builds
Expand All @@ -733,6 +740,12 @@ spec:
Default: 240 (4 hours)
format: int32
type: integer
maxBuildTTL:
description: |-
MaxBuildTTL is the maximum TTL that users can request for individual builds.
Build requests specifying a TTL greater than this value are rejected.
Set to "0" for no maximum. Uses Go duration format (e.g. "168h" for 1 week).
type: string
memoryVolumeSize:
description: |-
MemoryVolumeSize specifies the size limit for memory-backed volumes
Expand Down
7 changes: 7 additions & 0 deletions docs/openapi.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,9 @@ components:
type: array
items:
type: string
ttl:
type: string
description: 'Time-to-live for the build (e.g. "24h", "72h"). Empty uses server default, "0" disables expiry.'
BuildResponse:
type: object
properties:
Expand All @@ -179,6 +182,10 @@ components:
type: string
message:
type: string
expiresAt:
type: string
format: date-time
description: When the build will be automatically deleted (RFC 3339)
BuildListItem:
type: object
properties:
Expand Down
7 changes: 7 additions & 0 deletions internal/buildapi/openapi.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -218,6 +218,9 @@ components:
internalRegistryTag:
type: string
description: Tag for internal registry image (default is build name)
ttl:
type: string
description: 'Time-to-live for the build (e.g. "24h", "72h"). Empty uses server default, "0" disables expiry.'
BuildResponse:
type: object
properties:
Expand All @@ -230,6 +233,10 @@ components:
requestedBy:
type: string
nullable: true
expiresAt:
type: string
format: date-time
description: When the build will be automatically deleted (RFC 3339)
BuildListItem:
type: object
properties:
Expand Down
Loading
Loading