Skip to content
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

CRD usage of pointers vs non-pointers #945

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
5 changes: 5 additions & 0 deletions deploy/crds/shipwright.io_buildruns.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -758,6 +758,9 @@ spec:
description: Type of condition
type: string
required:
- lastTransitionTime
- message
- reason
- status
- type
type: object
Expand Down Expand Up @@ -850,6 +853,8 @@ spec:
format: date-time
type: string
type: object
required:
- spec
type: object
served: true
storage: true
Expand Down
2 changes: 2 additions & 0 deletions deploy/crds/shipwright.io_builds.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -431,6 +431,8 @@ spec:
description: The Register status of the Build
type: string
type: object
required:
- spec
type: object
served: true
storage: true
Expand Down
26 changes: 16 additions & 10 deletions pkg/apis/build/v1alpha1/build_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,16 @@ const (
AllValidationsSucceeded = "all validations succeeded"
)

// BuildReasonPtr returns a pointer to the passed BuildReason.
func BuildReasonPtr(s BuildReason) *BuildReason {
shahulsonhal marked this conversation as resolved.
Show resolved Hide resolved
return &s
}

// ConditionStatusPtr returns a pointer to the passed ConditionStatus.
func ConditionStatusPtr(s corev1.ConditionStatus) *corev1.ConditionStatus {
return &s
}

const (
// BuildDomain is the domain used for all labels and annotations for this resource
BuildDomain = "build.shipwright.io"
Expand Down Expand Up @@ -91,11 +101,11 @@ type BuildSpec struct {
// (`.spec.source`) data.
//
// +optional
Sources *[]BuildSource `json:"sources,omitempty"`
Sources []BuildSource `json:"sources,omitempty"`

// Strategy references the BuildStrategy to use to build the container
// image.
Strategy *Strategy `json:"strategy"`
Strategy Strategy `json:"strategy"`

// Builder refers to the image containing the build tools inside which
// the source code would be built.
Expand Down Expand Up @@ -137,10 +147,6 @@ func (buildSpec *BuildSpec) StrategyName() string {
return "undefined (nil buildSpec)"
}

if buildSpec.Strategy == nil {
return "undefined (nil strategy)"
}

return buildSpec.Strategy.Name
}

Expand Down Expand Up @@ -170,15 +176,15 @@ type Image struct {
type BuildStatus struct {
// The Register status of the Build
// +optional
Registered corev1.ConditionStatus `json:"registered,omitempty"`
Registered *corev1.ConditionStatus `json:"registered,omitempty"`
SaschaSchwarze0 marked this conversation as resolved.
Show resolved Hide resolved

// The reason of the registered Build, it's an one-word camelcase
// +optional
Reason BuildReason `json:"reason,omitempty"`
Reason *BuildReason `json:"reason,omitempty"`
SaschaSchwarze0 marked this conversation as resolved.
Show resolved Hide resolved

// The message of the registered Build, either an error or succeed message
// +optional
Message string `json:"message,omitempty"`
Message *string `json:"message,omitempty"`
SaschaSchwarze0 marked this conversation as resolved.
Show resolved Hide resolved
}

// +genclient
Expand All @@ -196,7 +202,7 @@ type Build struct {
metav1.TypeMeta `json:",inline"`
metav1.ObjectMeta `json:"metadata,omitempty"`

Spec BuildSpec `json:"spec,omitempty"`
Spec BuildSpec `json:"spec"`
SaschaSchwarze0 marked this conversation as resolved.
Show resolved Hide resolved
Status BuildStatus `json:"status,omitempty"`
}

Expand Down
32 changes: 17 additions & 15 deletions pkg/apis/build/v1alpha1/buildrun_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,13 +23,13 @@ const (
// BuildRunSpec defines the desired state of BuildRun
type BuildRunSpec struct {
// BuildRef refers to the Build
BuildRef *BuildRef `json:"buildRef"`
BuildRef BuildRef `json:"buildRef"`

// Sources slice of BuildSource, defining external build artifacts complementary to VCS
// (`.spec.source`) data.
//
// +optional
Sources *[]BuildSource `json:"sources,omitempty"`
Sources []BuildSource `json:"sources,omitempty"`

// ServiceAccount refers to the kubernetes serviceaccount
// which is used for resource control.
Expand All @@ -54,7 +54,7 @@ type BuildRunSpec struct {

// State is used for canceling a buildrun (and maybe more later on).
// +optional
State BuildRunRequestedState `json:"state,omitempty"`
State *BuildRunRequestedState `json:"state,omitempty"`
SaschaSchwarze0 marked this conversation as resolved.
Show resolved Hide resolved

// Env contains additional environment variables that should be passed to the build container
// +optional
Expand All @@ -64,6 +64,11 @@ type BuildRunSpec struct {
// BuildRunRequestedState defines the buildrun state the user can provide to override whatever is the current state.
type BuildRunRequestedState string

// BuildRunRequestedStatePtr returns a pointer to the passed BuildRunRequestedState.
func BuildRunRequestedStatePtr(s BuildRunRequestedState) *BuildRunRequestedState {
return &s
}

const (
// BuildRunStateCancel indicates that the user wants to cancel the BuildRun,
// if not already canceled or terminated
Expand Down Expand Up @@ -126,12 +131,12 @@ type BuildRunStatus struct {
// of different sources
//
// +optional
Sources []SourceResult `json:"sources"`
Sources []SourceResult `json:"sources,omitempty"`

// Output holds the results emitted from step definition of an output
//
// +optional
Output *Output `json:"output"`
Output *Output `json:"output,omitempty"`

// Conditions holds the latest available observations of a resource's current state.
Conditions Conditions `json:"conditions,omitempty"`
Expand Down Expand Up @@ -183,7 +188,7 @@ type BuildRef struct {
Name string `json:"name"`
// API version of the referent
// +optional
APIVersion string `json:"apiVersion,omitempty"`
APIVersion *string `json:"apiVersion,omitempty"`
SaschaSchwarze0 marked this conversation as resolved.
Show resolved Hide resolved
}

// ServiceAccount can be used to refer to a specific ServiceAccount.
Expand All @@ -193,7 +198,7 @@ type ServiceAccount struct {
Name *string `json:"name,omitempty"`
// If generates a new ServiceAccount for the build
// +optional
Generate bool `json:"generate,omitempty"`
Generate *bool `json:"generate,omitempty"`
SaschaSchwarze0 marked this conversation as resolved.
Show resolved Hide resolved
}

// +genclient
Expand All @@ -210,7 +215,7 @@ type BuildRun struct {
metav1.TypeMeta `json:",inline"`
metav1.ObjectMeta `json:"metadata,omitempty"`

Spec BuildRunSpec `json:"spec,omitempty"`
Spec BuildRunSpec `json:"spec"`
Status BuildRunStatus `json:"status,omitempty"`
}

Expand Down Expand Up @@ -242,7 +247,7 @@ func (br *BuildRun) IsSuccessful() bool {

// IsCanceled returns true if the BuildRun's spec status is set to BuildRunCanceled state.
func (br *BuildRun) IsCanceled() bool {
return br.Spec.State == BuildRunStateCancel
return br.Spec.State != nil && *br.Spec.State == BuildRunStateCancel
}

// Conditions defines a list of Condition
Expand All @@ -269,16 +274,13 @@ type Condition struct {
Status corev1.ConditionStatus `json:"status" description:"status of the condition, one of True, False, Unknown"`

// LastTransitionTime last time the condition transit from one status to another.
// +optional
LastTransitionTime metav1.Time `json:"lastTransitionTime,omitempty" description:"last time the condition transit from one status to another"`
LastTransitionTime metav1.Time `json:"lastTransitionTime" description:"last time the condition transit from one status to another"`

// The reason for the condition last transition.
// +optional
Reason string `json:"reason,omitempty" description:"one-word CamelCase reason for the condition's last transition"`
Reason string `json:"reason" description:"one-word CamelCase reason for the condition's last transition"`

// A human readable message indicating details about the transition.
// +optional
Message string `json:"message,omitempty" description:"human-readable message indicating details about last transition"`
Message string `json:"message" description:"human-readable message indicating details about last transition"`
}

func init() {
Expand Down
6 changes: 3 additions & 3 deletions pkg/apis/build/v1alpha1/buildstrategy.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ type Parameter struct {

// Default value for a string parameter
// +optional
Default *string `json:"default"`
Default *string `json:"default,omitempty"`

// Default values for an array parameter
// +optional
Expand All @@ -63,7 +63,7 @@ type Parameter struct {
// If the build step declares a volumeMount, Shipwright will create an emptyDir volume mount for the named volume.
// Build steps which share the same named volume in the volumeMount will share the same underlying emptyDir volume.
// This behavior is deprecated, and will be removed when full volume support is added to build strategies as specified
// in SHIP-0022.
// in SHIP-0022.
type BuildStep struct {
corev1.Container `json:",inline"`
}
Expand All @@ -86,7 +86,7 @@ type Strategy struct {

// API version of the referent
// +optional
APIVersion string `json:"apiVersion,omitempty"`
APIVersion *string `json:"apiVersion,omitempty"`
SaschaSchwarze0 marked this conversation as resolved.
Show resolved Hide resolved
}

// BuilderStrategy defines the common elements of build strategies
Expand Down
4 changes: 2 additions & 2 deletions pkg/apis/build/v1alpha1/source.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,12 @@ type Source struct {
// URL describes the URL of the Git repository.
//
// +optional
URL string `json:"url,omitempty"`
URL *string `json:"url,omitempty"`

// BundleContainer
//
// +optional
BundleContainer *BundleContainer `json:"bundleContainer"`
BundleContainer *BundleContainer `json:"bundleContainer,omitempty"`

// Revision describes the Git revision (e.g., branch, tag, commit SHA,
// etc.) to fetch.
Expand Down
74 changes: 49 additions & 25 deletions pkg/apis/build/v1alpha1/zz_generated.deepcopy.go

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

4 changes: 2 additions & 2 deletions pkg/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -84,8 +84,8 @@ var (
metricBuildRunEstablishDurationBuckets = []float64{0, 1, 2, 3, 5, 7, 10, 15, 20, 30}
metricBuildRunRampUpDurationBuckets = prometheus.LinearBuckets(0, 1, 10)

root = pointer.Int64Ptr(0)
nonRoot = pointer.Int64Ptr(1000)
root = pointer.Int64(0)
nonRoot = pointer.Int64(1000)
)

// Config hosts different parameters that
Expand Down
Loading