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

chore(api): mark PromotionMechanisms as deprecated #2594

Merged
merged 4 commits into from
Sep 28, 2024
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 api/v1alpha1/generated.proto

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

5 changes: 5 additions & 0 deletions api/v1alpha1/stage_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,9 @@ func (s *Stage) GetStatus() *StageStatus {

// StageSpec describes the sources of Freight used by a Stage and how to
// incorporate Freight into the Stage.
//
// +kubebuilder:validation:XValidation:rule="(has(self.promotionTemplate) || has(self.promotionMechanisms))",message="one of promotionTemplate or promotionMechanisms must be specified"
// +kubebuilder:validation:XValidation:rule="(has(self.promotionTemplate) && !has(self.promotionMechanisms)) || (!has(self.promotionTemplate) && has(self.promotionMechanisms))",message="only one of promotionTemplate or promotionMechanisms can be specified"
type StageSpec struct {
// Shard is the name of the shard that this Stage belongs to. This is an
// optional field. If not specified, the Stage will belong to the default
Expand Down Expand Up @@ -194,6 +197,8 @@ type StageSpec struct {
// utility of this is to allow multiple downstream Stages to subscribe to a
// single upstream Stage where they may otherwise have subscribed to multiple
// upstream Stages.
//
// Deprecated: Use PromotionTemplate instead.
PromotionMechanisms *PromotionMechanisms `json:"promotionMechanisms,omitempty" protobuf:"bytes,2,opt,name=promotionMechanisms"`
// Verification describes how to verify a Stage's current Freight is fit for
// promotion downstream.
Expand Down
10 changes: 10 additions & 0 deletions charts/kargo/resources/crds/kargo.akuity.io_stages.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,9 @@ spec:
utility of this is to allow multiple downstream Stages to subscribe to a
single upstream Stage where they may otherwise have subscribed to multiple
upstream Stages.


Deprecated: Use PromotionTemplate instead.
properties:
argoCDAppUpdates:
description: |-
Expand Down Expand Up @@ -1051,6 +1054,13 @@ spec:
required:
- requestedFreight
type: object
x-kubernetes-validations:
- message: one of promotionTemplate or promotionMechanisms must be specified
rule: (has(self.promotionTemplate) || has(self.promotionMechanisms))
- message: only one of promotionTemplate or promotionMechanisms can be
specified
rule: (has(self.promotionTemplate) && !has(self.promotionMechanisms))
|| (!has(self.promotionTemplate) && has(self.promotionMechanisms))
status:
description: Status describes the Stage's current and recent Freight,
health, and more.
Expand Down
2 changes: 1 addition & 1 deletion internal/api/promote_downstream_v1alpha1.go
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ func (s *server) PromoteDownstream(
createdPromos := make([]*kargoapi.Promotion, 0, len(downstreams))
for _, downstream := range downstreams {
newPromo := kargo.NewPromotion(ctx, downstream, freight.Name)
if downstream.Spec.PromotionMechanisms == nil {
if downstream.Spec.PromotionMechanisms == nil { // nolint: staticcheck
// Avoid creating a Promotion if the downstream Stage has no
// PromotionMechanisms, and is a "control flow" Stage.
continue
Expand Down
11 changes: 8 additions & 3 deletions internal/argocd/health.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ func (h *applicationHealth) EvaluateHealth(
ctx context.Context,
stage *kargoapi.Stage,
) *kargoapi.Health {
// nolint: staticcheck
if stage.Spec.PromotionMechanisms == nil ||
len(stage.Spec.PromotionMechanisms.ArgoCDAppUpdates) == 0 {
return nil
Expand All @@ -72,11 +73,15 @@ func (h *applicationHealth) EvaluateHealth(
}

health := kargoapi.Health{
Status: kargoapi.HealthStateHealthy,
ArgoCDApps: make([]kargoapi.ArgoCDAppStatus, len(stage.Spec.PromotionMechanisms.ArgoCDAppUpdates)),
Issues: make([]string, 0),
Status: kargoapi.HealthStateHealthy,
ArgoCDApps: make(
[]kargoapi.ArgoCDAppStatus,
len(stage.Spec.PromotionMechanisms.ArgoCDAppUpdates), // nolint: staticcheck
),
Issues: make([]string, 0),
}

// nolint: staticcheck
for i := range stage.Spec.PromotionMechanisms.ArgoCDAppUpdates {
update := &stage.Spec.PromotionMechanisms.ArgoCDAppUpdates[i]
namespace := update.AppNamespace
Expand Down
4 changes: 2 additions & 2 deletions internal/argocd/health_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -607,7 +607,7 @@ func TestApplicationHealth_GetApplicationHealth(t *testing.T) {
state, healthStatus, syncStatus, err := h.GetApplicationHealth(
context.Background(),
stage,
&stage.Spec.PromotionMechanisms.ArgoCDAppUpdates[0],
&stage.Spec.PromotionMechanisms.ArgoCDAppUpdates[0], // nolint: staticcheck
client.ObjectKey{
Namespace: app.Namespace,
Name: app.Name,
Expand Down Expand Up @@ -657,7 +657,7 @@ func TestApplicationHealth_GetApplicationHealth(t *testing.T) {
_, _, _, err := h.GetApplicationHealth(
context.Background(),
testStage,
&testStage.Spec.PromotionMechanisms.ArgoCDAppUpdates[0],
&testStage.Spec.PromotionMechanisms.ArgoCDAppUpdates[0], // nolint: staticcheck
client.ObjectKey{
Namespace: "fake-namespace",
Name: "fake-name",
Expand Down
2 changes: 1 addition & 1 deletion internal/argocd/revision_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ func TestGetDesiredRevisions(t *testing.T) {
context.Background(),
nil, // No client is needed as long as we're always explicit about origins
stage,
&stage.Spec.PromotionMechanisms.ArgoCDAppUpdates[0],
&stage.Spec.PromotionMechanisms.ArgoCDAppUpdates[0], // nolint: staticcheck
testCase.app,
testCase.freight,
)
Expand Down
2 changes: 1 addition & 1 deletion internal/controller/freight/origins.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ func getDesiredOriginInternal(
case *kargoapi.Stage:
// Stage is not technically a promotion mechanism, but it is a convenient
// entry point for the recursion.
subMechs = []any{m.Spec.PromotionMechanisms}
subMechs = []any{m.Spec.PromotionMechanisms} // nolint: staticcheck
case *kargoapi.PromotionMechanisms:
origin = m.Origin
subMechs = make([]any, len(m.GitRepoUpdates)+len(m.ArgoCDAppUpdates))
Expand Down
2 changes: 2 additions & 0 deletions internal/controller/freight/origins_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -334,6 +334,7 @@ func TestGetDesiredOrigin(t *testing.T) {
},
},
}
// nolint: staticcheck
return m, &m.Spec.PromotionMechanisms.GitRepoUpdates[0].Kustomize.Images[0]
},
},
Expand All @@ -353,6 +354,7 @@ func TestGetDesiredOrigin(t *testing.T) {
},
},
}
// nolint: staticcheck
return m, &m.Spec.PromotionMechanisms.GitRepoUpdates[0].Kustomize.Images[0]
},
},
Expand Down
2 changes: 1 addition & 1 deletion internal/controller/promotion/argocd.go
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ func (a *argoCDMechanism) Promote(
stage *kargoapi.Stage,
promo *kargoapi.Promotion,
) error {
updates := stage.Spec.PromotionMechanisms.ArgoCDAppUpdates
updates := stage.Spec.PromotionMechanisms.ArgoCDAppUpdates // nolint: staticcheck

if len(updates) == 0 {
promo.Status.Phase = kargoapi.PromotionPhaseSucceeded
Expand Down
10 changes: 5 additions & 5 deletions internal/controller/promotion/argocd_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -813,7 +813,7 @@ func TestArgoCDBuildDesiredSources(t *testing.T) {
desiredSources, err := testCase.reconciler.buildDesiredSources(
context.Background(),
stage,
&stage.Spec.PromotionMechanisms.ArgoCDAppUpdates[0],
&stage.Spec.PromotionMechanisms.ArgoCDAppUpdates[0], // nolint: staticcheck
app,
[]kargoapi.FreightReference{},
)
Expand Down Expand Up @@ -1164,7 +1164,7 @@ func TestArgoCDMustPerformUpdate(t *testing.T) {
phase, mustUpdate, err := argocdMech.mustPerformUpdate(
context.Background(),
stage,
&stage.Spec.PromotionMechanisms.ArgoCDAppUpdates[0],
&stage.Spec.PromotionMechanisms.ArgoCDAppUpdates[0], // nolint: staticcheck
app,
freight,
testCase.desiredSources,
Expand Down Expand Up @@ -1841,7 +1841,7 @@ func TestApplyArgoCDSourceUpdate(t *testing.T) {
updatedSource, err := mech.applyArgoCDSourceUpdate(
context.Background(),
stage,
&stage.Spec.PromotionMechanisms.ArgoCDAppUpdates[0].SourceUpdates[0],
&stage.Spec.PromotionMechanisms.ArgoCDAppUpdates[0].SourceUpdates[0], // nolint: staticcheck
testCase.source,
testCase.freight,
)
Expand Down Expand Up @@ -1895,7 +1895,7 @@ func TestBuildKustomizeImagesForArgoCDAppSource(t *testing.T) {
result, err := mech.buildKustomizeImagesForArgoCDAppSource(
context.Background(),
stage,
stage.Spec.PromotionMechanisms.ArgoCDAppUpdates[0].SourceUpdates[0].Kustomize,
stage.Spec.PromotionMechanisms.ArgoCDAppUpdates[0].SourceUpdates[0].Kustomize, // nolint: staticcheck
freight,
)
require.NoError(t, err)
Expand Down Expand Up @@ -1984,7 +1984,7 @@ func TestBuildHelmParamChangesForArgoCDAppSource(t *testing.T) {
result, err := mech.buildHelmParamChangesForArgoCDAppSource(
context.Background(),
stage,
stage.Spec.PromotionMechanisms.ArgoCDAppUpdates[0].SourceUpdates[0].Helm,
stage.Spec.PromotionMechanisms.ArgoCDAppUpdates[0].SourceUpdates[0].Helm, // nolint: staticcheck
freight,
)
require.NoError(t, err)
Expand Down
2 changes: 1 addition & 1 deletion internal/controller/promotion/composite.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ func (c *compositeMechanism) Promote(
stage *kargoapi.Stage,
promo *kargoapi.Promotion,
) error {
if stage.Spec.PromotionMechanisms == nil {
if stage.Spec.PromotionMechanisms == nil { // nolint: staticcheck
promo.Status.Phase = kargoapi.PromotionPhaseSucceeded
return nil
}
Expand Down
1 change: 1 addition & 0 deletions internal/controller/promotion/git.go
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,7 @@ func (g *gitMechanism) Promote(
stage *kargoapi.Stage,
promo *kargoapi.Promotion,
) error {
// nolint: staticcheck
updates := g.selectUpdatesFn(stage.Spec.PromotionMechanisms.GitRepoUpdates)

if len(updates) == 0 {
Expand Down
2 changes: 1 addition & 1 deletion internal/controller/promotion/git_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -478,7 +478,7 @@ func TestGetReadRef(t *testing.T) {
context.Background(),
fake.NewFakeClient(),
stage,
&stage.Spec.PromotionMechanisms.GitRepoUpdates[0],
&stage.Spec.PromotionMechanisms.GitRepoUpdates[0], // nolint: staticcheck
testCase.freight,
)
testCase.assertions(t, readBranch, commit, err)
Expand Down
5 changes: 3 additions & 2 deletions internal/controller/promotion/helm_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -309,6 +309,7 @@ func TestHelmerApply(t *testing.T) {
},
},
}
// nolint: staticcheck
changes, err := testCase.helmer.apply(
context.Background(),
stage,
Expand Down Expand Up @@ -376,7 +377,7 @@ func TestBuildValuesFilesChanges(t *testing.T) {
result, changeSummary, err := h.buildValuesFilesChanges(
context.Background(),
stage,
stage.Spec.PromotionMechanisms.GitRepoUpdates[0].Helm,
stage.Spec.PromotionMechanisms.GitRepoUpdates[0].Helm, // nolint: staticcheck
[]kargoapi.FreightReference{{
Origin: testOrigin,
Images: []kargoapi.Image{
Expand Down Expand Up @@ -525,7 +526,7 @@ func TestBuildChartDependencyChanges(t *testing.T) {
result, changeSummary, err := h.buildChartDependencyChanges(
context.Background(),
stage,
stage.Spec.PromotionMechanisms.GitRepoUpdates[0].Helm,
stage.Spec.PromotionMechanisms.GitRepoUpdates[0].Helm, // nolint: staticcheck
freight,
testDir,
)
Expand Down
2 changes: 1 addition & 1 deletion internal/controller/promotion/kustomize_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -239,7 +239,7 @@ func TestKustomizerApply(t *testing.T) {
changes, err := testCase.kustomizer.apply(
context.Background(),
stage,
&stage.Spec.PromotionMechanisms.GitRepoUpdates[0],
&stage.Spec.PromotionMechanisms.GitRepoUpdates[0], // nolint: staticcheck
[]kargoapi.FreightReference{{
Images: []kargoapi.Image{
{
Expand Down
2 changes: 1 addition & 1 deletion internal/controller/promotion/render_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -239,7 +239,7 @@ func TestKargoRenderApply(t *testing.T) {
changes, err := testCase.renderer.apply(
context.Background(),
stage,
&stage.Spec.PromotionMechanisms.GitRepoUpdates[0],
&stage.Spec.PromotionMechanisms.GitRepoUpdates[0], // nolint: staticcheck
testCase.newFreight,
testSourceCommitID,
"", // Home directory is not used by this implementation
Expand Down
6 changes: 6 additions & 0 deletions internal/kubeclient/indexer.go
Original file line number Diff line number Diff line change
Expand Up @@ -155,10 +155,13 @@ func indexStagesByArgoCDApplications(shardName string) client.IndexerFunc {
}

stage := obj.(*kargoapi.Stage) // nolint: forcetypeassert
// nolint: staticcheck
if stage.Spec.PromotionMechanisms == nil || len(stage.Spec.PromotionMechanisms.ArgoCDAppUpdates) == 0 {
return nil
}
// nolint: staticcheck
apps := make([]string, len(stage.Spec.PromotionMechanisms.ArgoCDAppUpdates))
// nolint: staticcheck
for i, appCheck := range stage.Spec.PromotionMechanisms.ArgoCDAppUpdates {
namespace := appCheck.AppNamespace
if namespace == "" {
Expand Down Expand Up @@ -280,13 +283,16 @@ func indexRunningPromotionsByArgoCDApplications(
return nil
}

// nolint: staticcheck
if stage.Spec.PromotionMechanisms == nil || len(stage.Spec.PromotionMechanisms.ArgoCDAppUpdates) == 0 {
// If the Stage has no Argo CD Application promotion mechanisms,
// then we have nothing to index.
return nil
}

// nolint: staticcheck
res := make([]string, len(stage.Spec.PromotionMechanisms.ArgoCDAppUpdates))
// nolint: staticcheck
for i, appUpdate := range stage.Spec.PromotionMechanisms.ArgoCDAppUpdates {
namespace := appUpdate.AppNamespace
if namespace == "" {
Expand Down
1 change: 1 addition & 0 deletions internal/webhook/promotion/webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,7 @@ func (w *webhook) Default(ctx context.Context, obj runtime.Object) error {
promo.Namespace,
)
}
// nolint: staticcheck
if len(promo.Spec.Steps) == 0 && stage.Spec.PromotionMechanisms == nil {
return fmt.Errorf(
"Stage %q in namespace %q has no PromotionMechanisms",
Expand Down
2 changes: 1 addition & 1 deletion internal/webhook/stage/webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,7 @@ func (w *webhook) validateSpec(
errs,
w.validatePromotionMechanisms(
f.Child("promotionMechanisms"),
spec.PromotionMechanisms,
spec.PromotionMechanisms, // nolint: staticcheck
)...,
)
}
Expand Down
2 changes: 1 addition & 1 deletion internal/webhook/stage/webhook_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1053,7 +1053,7 @@ func TestValidateSpec(t *testing.T) {
{
Type: field.ErrorTypeInvalid,
Field: "spec.promotionMechanisms",
BadValue: spec.PromotionMechanisms,
BadValue: spec.PromotionMechanisms, // nolint: staticcheck
Detail: "at least one of " +
"spec.promotionMechanisms.gitRepoUpdates or " +
"spec.promotionMechanisms.argoCDAppUpdates must be non-empty",
Expand Down
14 changes: 12 additions & 2 deletions ui/src/gen/schema/stages.kargo.akuity.io_v1alpha1.json
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
"description": "Spec describes sources of Freight used by the Stage and how to incorporate\nFreight into the Stage.",
"properties": {
"promotionMechanisms": {
"description": "PromotionMechanisms describes how to incorporate Freight into the Stage.\nThis is an optional field as it is sometimes useful to aggregates available\nFreight from multiple upstream Stages without performing any actions. The\nutility of this is to allow multiple downstream Stages to subscribe to a\nsingle upstream Stage where they may otherwise have subscribed to multiple\nupstream Stages.",
"description": "PromotionMechanisms describes how to incorporate Freight into the Stage.\nThis is an optional field as it is sometimes useful to aggregates available\nFreight from multiple upstream Stages without performing any actions. The\nutility of this is to allow multiple downstream Stages to subscribe to a\nsingle upstream Stage where they may otherwise have subscribed to multiple\nupstream Stages.\n\n\nDeprecated: Use PromotionTemplate instead.",
"properties": {
"argoCDAppUpdates": {
"description": "ArgoCDAppUpdates describes updates that should be applied to Argo CD\nApplication resources to incorporate Freight into the Stage. This field is\noptional, as such actions are not required in all cases. Note that all\nupdates specified by the GitRepoUpdates field, if any, are applied BEFORE\nthese.",
Expand Down Expand Up @@ -832,7 +832,17 @@
"required": [
"requestedFreight"
],
"type": "object"
"type": "object",
"x-kubernetes-validations": [
{
"message": "one of promotionTemplate or promotionMechanisms must be specified",
"rule": "(has(self.promotionTemplate) || has(self.promotionMechanisms))"
},
{
"message": "only one of promotionTemplate or promotionMechanisms can be specified",
"rule": "(has(self.promotionTemplate) && !has(self.promotionMechanisms)) || (!has(self.promotionTemplate) && has(self.promotionMechanisms))"
}
]
},
"status": {
"description": "Status describes the Stage's current and recent Freight, health, and more.",
Expand Down
5 changes: 5 additions & 0 deletions ui/src/gen/v1alpha1/generated_pb.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4439,6 +4439,9 @@ export class StageList extends Message<StageList> {
* StageSpec describes the sources of Freight used by a Stage and how to
* incorporate Freight into the Stage.
*
* +kubebuilder:validation:XValidation:rule="(has(self.promotionTemplate) || has(self.promotionMechanisms))",message="one of promotionTemplate or promotionMechanisms must be specified"
* +kubebuilder:validation:XValidation:rule="(has(self.promotionTemplate) && !has(self.promotionMechanisms)) || (!has(self.promotionTemplate) && has(self.promotionMechanisms))",message="only one of promotionTemplate or promotionMechanisms can be specified"
*
* @generated from message git.luolix.top.akuity.kargo.api.v1alpha1.StageSpec
*/
export class StageSpec extends Message<StageSpec> {
Expand Down Expand Up @@ -4485,6 +4488,8 @@ export class StageSpec extends Message<StageSpec> {
* single upstream Stage where they may otherwise have subscribed to multiple
* upstream Stages.
*
* Deprecated: Use PromotionTemplate instead.
*
* @generated from field: optional git.luolix.top.akuity.kargo.api.v1alpha1.PromotionMechanisms promotionMechanisms = 2;
*/
promotionMechanisms?: PromotionMechanisms;
Expand Down