-
Notifications
You must be signed in to change notification settings - Fork 9
feat: add SoftwareBuild CRD for multi-OS software builds #199
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
d57de13
42d85c2
503fc61
aa58e7c
58857c6
07b76c4
b8127f1
43b22c7
fc61e8f
ba43d8c
4ae687a
0675cb1
e257f24
ff03f8c
c49f982
3db7494
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,192 @@ | ||
| /* | ||
| Copyright 2025. | ||
|
|
||
| Licensed under the Apache License, Version 2.0 (the "License"); | ||
| you may not use this file except in compliance with the License. | ||
| You may obtain a copy of the License at | ||
|
|
||
| http://www.apache.org/licenses/LICENSE-2.0 | ||
|
|
||
| Unless required by applicable law or agreed to in writing, software | ||
| distributed under the License is distributed on an "AS IS" BASIS, | ||
| WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| See the License for the specific language governing permissions and | ||
| limitations under the License. | ||
| */ | ||
|
|
||
| package v1alpha1 | ||
|
|
||
| import ( | ||
| metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" | ||
| ) | ||
|
|
||
| // SoftwareBuildSourceType identifies where source code is obtained from. | ||
| type SoftwareBuildSourceType string | ||
|
|
||
| // Source can be a Git repository or an existing PVC with pre-populated content. | ||
| const ( | ||
| SoftwareBuildSourceGit SoftwareBuildSourceType = "git" | ||
| SoftwareBuildSourcePVC SoftwareBuildSourceType = "pvc" | ||
| ) | ||
|
|
||
| // SoftwareBuildDestinationType identifies where build artifacts are stored. | ||
| type SoftwareBuildDestinationType string | ||
|
|
||
| // Artifacts are written to a shared folder on the workspace PVC. | ||
| const ( | ||
| SoftwareBuildDestinationSharedFolder SoftwareBuildDestinationType = "sharedFolder" | ||
| ) | ||
|
|
||
| // SoftwareBuildPhase represents the current lifecycle phase. | ||
| type SoftwareBuildPhase string | ||
|
|
||
| // Phases track the SoftwareBuild lifecycle from submission through completion. | ||
| const ( | ||
| SoftwareBuildPhasePending SoftwareBuildPhase = "Pending" | ||
| SoftwareBuildPhaseRunning SoftwareBuildPhase = "Running" | ||
| SoftwareBuildPhaseSucceeded SoftwareBuildPhase = "Succeeded" | ||
| SoftwareBuildPhaseFailed SoftwareBuildPhase = "Failed" | ||
| ) | ||
|
|
||
| // SoftwareBuildRuntimeSpec configures the container environment used for every | ||
| // pipeline stage unless overridden per-stage. | ||
| type SoftwareBuildRuntimeSpec struct { | ||
| // Image is the container image that provides the build toolchain. | ||
| // +kubebuilder:default="ubuntu:24.04" | ||
|
Comment on lines
+54
to
+55
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [MEDIUM] Container image tags are mutable, undermining build reproducibility. Suggestion: Resolve tags to digests at reconciliation time and record the resolved digest in status for reproducibility.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Partially agreed. For v1, will record the resolved image digest in status at reconciliation time for reproducibility. Enforcing digest-only inputs would be too restrictive for the UX. Full image digest resolution is tracked in #200. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Tracking it does not resolve the problem. I would like to see this addressed before merging but am also curious about @bennyz thoughts on this. I think we have workspaces to run arbitrary commands - pipelines should be deterministic.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Again, this code is not final! It's the first implementation. I reiterate that this will be improved with #200 (and #197). long version: "Pipelines should be deterministic" applies at the CI/CD level, where the commands are checked into source control (the SoftwareBuild CR YAML). The CRD doesn't make them less deterministic than a Tekton Pipeline with inline scripts — it structures them into well-known stages with per-stage image override, timeout, and lifecycle tracking. Workspaces serve a different purpose: interactive development environments. A build pipeline needs reproducible, ephemeral execution with artifact outputs — which is what SoftwareBuild provides. That said, when Tekton Chains integration lands (#200), command execution will happen inside signed task bundles with SLSA provenance, giving us auditability even for arbitrary commands. |
||
| // +kubebuilder:validation:MinLength=1 | ||
| Image string `json:"image,omitempty"` | ||
|
|
||
| // ServiceAccountName is the Kubernetes SA the build pod runs as. | ||
| // +optional | ||
| ServiceAccountName string `json:"serviceAccountName,omitempty"` | ||
|
Comment on lines
+60
to
+61
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [LOW] Suggestion: Wire the field when non-empty so that builds can run under a user-specified service account.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Agreed. Will wire There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The original issue seems resolved but ServiceAccountName now allows arbitrary SA reference without validation or documentation. The optional ServiceAccountName field is propagated directly to the PipelineRun's TaskRunTemplate.ServiceAccountName with no validation. A user with SoftwareBuild RBAC can reference a high-privilege SA, and the build container runs with that SA's token. This is inherent to any CRD that creates pods/PipelineRuns. Suggestion: document that granting SoftwareBuild creation RBAC is equivalent to granting pod creation rights within the target namespace and an allowlist in OperatorConfig.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I suggest tracking this as a bug of documentation and this will get fixed as soon as this lands into main.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good point. This is inherent to any CRD that creates pods on behalf of users — granting SoftwareBuild RBAC is equivalent to granting pod creation rights in the namespace. We can document this in the CRD's godoc and add an allowlist in OperatorConfig as a follow-up. The current behavior is identical to how ImageBuild handles SA references. |
||
| } | ||
|
|
||
| // SoftwareBuildGitSource describes a Git repository to clone. | ||
| type SoftwareBuildGitSource struct { | ||
| // +kubebuilder:validation:Pattern=`^https?://[^\s;|&$'"]+$` | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [HIGH] Git URL validation pattern does not block backtick, enabling command substitution. The CRD URL validation pattern Suggestion: add backtick to the exclusion set. Also consider blocking AI-generated, human reviewed |
||
| URL string `json:"url"` | ||
| // +kubebuilder:validation:Pattern=`^[a-zA-Z0-9._/-]+$` | ||
| // +kubebuilder:default=main | ||
| Revision string `json:"revision,omitempty"` | ||
| } | ||
|
|
||
| // SoftwareBuildPVCSource references an existing PVC. | ||
| // +kubebuilder:validation:XValidation:rule="!has(self.path) || !self.path.contains('..')",message="path must not contain '..'" | ||
| type SoftwareBuildPVCSource struct { | ||
| // +kubebuilder:validation:MinLength=1 | ||
| ClaimName string `json:"claimName"` | ||
| // +kubebuilder:default=/ | ||
| Path string `json:"path,omitempty"` | ||
|
coderabbitai[bot] marked this conversation as resolved.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [MEDIUM] PVC source path field lacks CRD-level path-traversal validation. The Suggestion: add a CEL validation rule: AI-generated, human reviewed
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed in 3db7494: added |
||
| } | ||
|
|
||
| // SoftwareBuildSourceSpec identifies the code location. | ||
| // +kubebuilder:validation:XValidation:rule="self.type != 'git' || has(self.git)",message="git source details required when type is git" | ||
| // +kubebuilder:validation:XValidation:rule="self.type != 'pvc' || has(self.pvc)",message="pvc source details required when type is pvc" | ||
| type SoftwareBuildSourceSpec struct { | ||
| // +kubebuilder:validation:Enum=git;pvc | ||
| Type SoftwareBuildSourceType `json:"type"` | ||
| // +optional | ||
| Git *SoftwareBuildGitSource `json:"git,omitempty"` | ||
| // +optional | ||
| PVC *SoftwareBuildPVCSource `json:"pvc,omitempty"` | ||
| } | ||
|
Comment on lines
+85
to
+92
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [HIGH] No input validation for source type and source field consistency. The CRD allows setting Suggestion: Add CEL validation rules, e.g.,
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Agreed. Will add CEL validation rules: There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. cant resolve but i think it looks good |
||
|
|
||
| // SoftwareBuildStageSpec defines a single pipeline stage. | ||
| type SoftwareBuildStageSpec struct { | ||
| // Command is executed via bash inside the runtime image. | ||
| // +kubebuilder:validation:MinLength=1 | ||
| Command string `json:"command"` | ||
| // Image overrides the runtime image for this stage only. | ||
| // +optional | ||
| Image string `json:"image,omitempty"` | ||
|
Comment on lines
+100
to
+101
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [MEDIUM] Per-stage image override is declared but not wired. Suggestion: Wire the per-stage image: if
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Agreed. Will wire the per-stage image: if |
||
| } | ||
|
|
||
| // SoftwareBuildPipelineStages groups the five sequential stages. | ||
| type SoftwareBuildPipelineStages struct { | ||
| Fetch SoftwareBuildStageSpec `json:"fetch"` | ||
| Prebuild SoftwareBuildStageSpec `json:"prebuild"` | ||
| Build SoftwareBuildStageSpec `json:"build"` | ||
| Postbuild SoftwareBuildStageSpec `json:"postbuild"` | ||
| Deploy SoftwareBuildStageSpec `json:"deploy"` | ||
| } | ||
|
Comment on lines
+104
to
+111
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [MEDIUM] Five fixed stages is an inflexible design that requires an API change to extend. Suggestion: Consider using a
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Partially agreed. The fixed stages provide a simple, opinionated workflow for v1. Will add There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. i think thats okay but the root cause #199 (comment) remains unanswered. |
||
|
|
||
| // SoftwareBuildDestinationSpec describes where artifacts go. | ||
| type SoftwareBuildDestinationSpec struct { | ||
| // +kubebuilder:validation:Enum=sharedFolder | ||
| Type SoftwareBuildDestinationType `json:"type"` | ||
| // +optional | ||
| Path string `json:"path,omitempty"` | ||
| } | ||
|
|
||
| // SoftwareBuildSpec defines the desired state of SoftwareBuild. | ||
| type SoftwareBuildSpec struct { | ||
| // +optional | ||
| Runtime SoftwareBuildRuntimeSpec `json:"runtime,omitempty"` | ||
| Source SoftwareBuildSourceSpec `json:"source"` | ||
| Stages SoftwareBuildPipelineStages `json:"stages"` | ||
| Destination SoftwareBuildDestinationSpec `json:"destination"` | ||
| // +kubebuilder:validation:Minimum=0 | ||
| // +optional | ||
| TimeoutSeconds int64 `json:"timeoutSeconds,omitempty"` | ||
| } | ||
|
coderabbitai[bot] marked this conversation as resolved.
|
||
|
|
||
| // SoftwareBuildStageStatus captures per-stage progress. | ||
| type SoftwareBuildStageStatus struct { | ||
| Name string `json:"name,omitempty"` | ||
| // +optional | ||
| StartedAt *metav1.Time `json:"startedAt,omitempty"` | ||
| // +optional | ||
| FinishedAt *metav1.Time `json:"finishedAt,omitempty"` | ||
| // +optional | ||
| State string `json:"state,omitempty"` | ||
| // +optional | ||
| Message string `json:"message,omitempty"` | ||
| } | ||
|
|
||
| // SoftwareBuildStatus defines the observed state of SoftwareBuild. | ||
| type SoftwareBuildStatus struct { | ||
| // +optional | ||
| Phase SoftwareBuildPhase `json:"phase,omitempty"` | ||
| // +optional | ||
| PipelineRunName string `json:"pipelineRunName,omitempty"` | ||
| // +optional | ||
| ArtifactURI string `json:"artifactURI,omitempty"` | ||
| // +optional | ||
| FailureReason string `json:"failureReason,omitempty"` | ||
| // +optional | ||
| Stages []SoftwareBuildStageStatus `json:"stages,omitempty"` | ||
| // +optional | ||
| Conditions []metav1.Condition `json:"conditions,omitempty"` | ||
| } | ||
|
|
||
| // +kubebuilder:object:root=true | ||
| // +kubebuilder:subresource:status | ||
| // +kubebuilder:resource:path=softwarebuilds,scope=Namespaced,shortName=sb | ||
| // +kubebuilder:printcolumn:name="Phase",type=string,JSONPath=`.status.phase` | ||
| // +kubebuilder:printcolumn:name="Image",type=string,JSONPath=`.spec.runtime.image` | ||
| // +kubebuilder:printcolumn:name="Age",type=date,JSONPath=`.metadata.creationTimestamp` | ||
|
|
||
| // SoftwareBuild is the Schema for the softwarebuilds API. | ||
| // It drives a generic, stage-based Tekton pipeline that can build software | ||
| // for any target OS or toolchain by specifying a runtime container image and | ||
| // five sequential shell commands. | ||
| type SoftwareBuild struct { | ||
| metav1.TypeMeta `json:",inline"` | ||
| metav1.ObjectMeta `json:"metadata,omitempty"` | ||
|
|
||
| Spec SoftwareBuildSpec `json:"spec,omitempty"` | ||
| Status SoftwareBuildStatus `json:"status,omitempty"` | ||
| } | ||
|
|
||
| // +kubebuilder:object:root=true | ||
|
|
||
| // SoftwareBuildList contains a list of SoftwareBuild. | ||
| type SoftwareBuildList struct { | ||
| metav1.TypeMeta `json:",inline"` | ||
| metav1.ListMeta `json:"metadata,omitempty"` | ||
| Items []SoftwareBuild `json:"items"` | ||
| } | ||
|
|
||
| func init() { | ||
| SchemeBuilder.Register(&SoftwareBuild{}, &SoftwareBuildList{}) | ||
| } | ||
|
Comment on lines
+1
to
+192
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [HIGH] The SoftwareBuild CRD adds no domain-specific value over raw Tekton Pipelines. Five hardcoded stages map 1:1 to Tekton tasks, source types map to workspace bindings, and unlike ImageBuild, SoftwareBuild adds no domain-specific transformation. The CRD creates a lossy abstraction that limits expressiveness while complicating Tekton Chains integration. Suggestion: Consider whether SoftwareBuild should be a Tekton custom task, a pipeline template, or a set of pre-built task bundles. If kept as a CRD, add automotive-domain-specific value that justifies the abstraction.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I respectfully disagree that this adds no domain-specific value. The design rationale:
That said, the current implementation is intentionally minimal for v1. The abstraction will grow in value as we add these features. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This AI generated response does not address the actual point. ImageBuild validates distro/target/mode, has mode-specific logic, orchestrates exports, manages hardware flashing, tracks provenance. SoftwareBuild passes a string to bash. Calling this "the same pattern" is wrong. One transforms, the other proxies. The target audience argument would hold if the CRD gave them something but it does not. Users still write raw shell commands and debug their own failures. The CRD hides YAML syntax, which a Pipeline template, a shell script wrapping tkn or Helm chart also does without introducing a new API surface to maintain. "Operator-managed lifecycle" is what every controller does by definition. That is not domain-specific value, that is the baseline. Five stages map 1:1 to five tasks, source types map 1:1 to tekton workspace bindings. TL;DR: None of these facts from the original review were disputed or resolved hence I am asking for further changes and discussion.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. TL;DR: Tekton alone covers what this PR does today. It won't cover what RHAS needs tomorrow — shared caches, compliance gating, AI assistance, a developer CLI, and role separation all require an abstraction above Tekton. This CRD is that abstraction. You're right. Today, SoftwareBuild is a thin wrapper around Tekton. Five stages, five tasks, 218 lines. No transformation, no domain logic. I won't argue otherwise. But this PR ships the API contract, not the finished product. Raw Tekton would be enough for what this PR does today. It won't be enough for what we're building:
The domain features — caches, compliance, AI, build tiers — are on the roadmap, not yet implemented. The CRD lands first so that downstream work (API endpoints, CLI commands, cache management) has a stable contract to build against. That's a sequencing decision, not an absence of vision. |
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[MEDIUM] Four destination types (
sharedFolder,registry,artifactory,quay) are defined but onlysharedFolderis partially used. Unused enum values create a false impression of supported functionality.Suggestion: Reduce the enum to only
sharedFolderfor this iteration and add others when they are actually implemented.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed. Will remove
registry,artifactory, andquayfrom the enum — onlysharedFolderis implemented.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
seems resolved, but I can not resolve comment maybe because you removed me as a reviewer?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You should have reviewer access now. Let me know if you still can't resolve — it might be a GitHub permissions issue on the fork PR.