Skip to content

Commit

Permalink
feat(STONEINTG-966): ensure override snapshot is valid to be released
Browse files Browse the repository at this point in the history 
* Validate Override snapshot to release it and mark it as invalid if it
  doesn't follow:
  the snapshotComponent should exist as component
  containerImage in snapshotComponent has valid digest
  snapshotComponent has git source defined

Signed-off-by: Hongwei Liu <[email protected]>
  • Loading branch information
@hongweiliu17
hongweiliu17 committed Jul 4, 2024
1 parent 2f08ad5 commit 1d8a6a3
Show file tree
Hide file tree
Showing 6 changed files with 185 additions and 11 deletions.
22 changes: 11 additions & 11 deletions docs/snapshot-controller.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -80,24 +80,24 @@ flowchart TD
encountered_error32 --Yes--> mark_snapshot_Invalid3
%%%%%%%%%%%%%%%%%%%%%%% Drawing EnsureRerunPipelineRunsExist() function
%%%%%%%%%%%%%%%%%%%%%%% Drawing EnsureOverrideSnapshotValid() function
%% Node definitions
ensure6(Process further if: Snapshot has re-run label added by a user)
if_scenario_exist{Does scenario requested by user exist?}
remove_rerun_label(Remove rerun label)
rerun_static_env(Rerun static env pipeline for scenario)
ensure4(Process further if: Snapshot has override type label)
validate_override_valid{Check if override snapshot has <br>valid image digest and git source}
mark_snapshot_invalid(<b>Mark</b> overide snapshot as invalid)
continue_processing6(Controller continues processing...)
%% Node connections
predicate ----> |"EnsureRerunPipelineRunsExist()"|ensure6
ensure6 --> if_scenario_exist
if_scenario_exist --Yes--> rerun_static_env
if_scenario_exist --No--> remove_rerun_label
remove_rerun_label ----> continue_processing6
rerun_static_env ----> remove_rerun_label
predicate ----> |"EnsureOverrideSnapshotValid()"|ensure4
ensure4 --> validate_override_valid
validate_override_valid --Yes--> continue_processing6
validate_override_valid --No--> mark_snapshot_invalid
mark_snapshot_invalid --> continue_processing6
%%%%%%%%%%%%%%%%%%%%%%% Drawing EnsureIntegrationPipelineRunsExist() function
%% Assigning styles to nodes
class predicate Amber;
class encountered_error1,encountered_error31,encountered_error32,encountered_error5 Red;
Expand Down
19 changes: 19 additions & 0 deletions gitops/snapshot.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -473,6 +473,12 @@ func ValidateImageDigest(imageUrl string) error {
return err
}

// IsGitSourceValid checks if snapshotComponent contains valid git source
func IsGitSourceValid(snapshotComponent applicationapiv1alpha1.SnapshotComponent) bool {
return reflect.ValueOf(snapshotComponent.Source).IsValid() && snapshotComponent.Source.GitSource != nil &&
snapshotComponent.Source.GitSource.Revision != "" && snapshotComponent.Source.GitSource.URL != ""
}

// HaveAppStudioTestsFinished checks if the AppStudio tests have finished by checking if the AppStudio Test Succeeded condition is set.
func HaveAppStudioTestsFinished(snapshot *applicationapiv1alpha1.Snapshot) bool {
statusCondition := meta.FindStatusCondition(snapshot.Status.Conditions, AppStudioTestSucceededCondition)
Expand Down Expand Up @@ -864,3 +870,16 @@ func IsComponentSnapshot(snapshot *applicationapiv1alpha1.Snapshot) bool {
func IsComponentSnapshotCreatedByPACPushEvent(snapshot *applicationapiv1alpha1.Snapshot) bool {
return IsComponentSnapshot(snapshot) && IsSnapshotCreatedByPACPushEvent(snapshot)
}

func SetOwnerReference(ctx context.Context, adapterClient client.Client, snapshot *applicationapiv1alpha1.Snapshot, application *applicationapiv1alpha1.Application) (*applicationapiv1alpha1.Snapshot, error) {
patch := client.MergeFrom(snapshot.DeepCopy())
err := ctrl.SetControllerReference(application, snapshot, adapterClient.Scheme())
if err != nil {
return snapshot, err
}
err = adapterClient.Patch(ctx, snapshot, patch)
if err != nil {
return snapshot, err
}
return snapshot, nil
}
8 changes: 8 additions & 0 deletions gitops/snapshot_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ import (
"k8s.io/apimachinery/pkg/api/meta"
"k8s.io/apimachinery/pkg/types"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"

metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
)
Expand Down Expand Up @@ -608,12 +609,19 @@ var _ = Describe("Gitops functions for managing Snapshots", Ordered, func() {
BeforeEach(func() {
overrideSnapshot = hasSnapshot.DeepCopy()
overrideSnapshot.Labels[gitops.SnapshotTypeLabel] = gitops.SnapshotOverrideType
Expect(controllerutil.HasControllerReference(overrideSnapshot)).To(BeFalse())
})

It("make sure correct label is returned in overrideSnapshot", func() {
isOverrideSnapshot := gitops.IsOverrideSnapshot(overrideSnapshot)
Expect(isOverrideSnapshot).To(BeTrue())
})

It("Can set owner reference for override snapshot", func() {
overrideSnapshot, err := gitops.SetOwnerReference(ctx, k8sClient, overrideSnapshot, hasApp)
Expect(controllerutil.HasControllerReference(overrideSnapshot)).To(BeTrue())
Expect(err).ToNot(HaveOccurred())
})
})

})
Expand Down
67 changes: 67 additions & 0 deletions internal/controller/snapshot/snapshot_adapter.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import (
clienterrors "k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/api/meta"
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"

"github.com/konflux-ci/integration-service/api/v1beta2"
"github.com/konflux-ci/integration-service/gitops"
Expand Down Expand Up @@ -385,6 +386,7 @@ func (a *Adapter) EnsureAllReleasesExist() (controller.OperationResult, error) {
"reasons", strings.Join(reasons, ","))
return controller.ContinueProcessing()
}

if gitops.IsSnapshotMarkedAsAutoReleased(a.snapshot) {
a.logger.Info("The Snapshot was previously auto-released, skipping auto-release.")
return controller.ContinueProcessing()
Expand Down Expand Up @@ -433,6 +435,71 @@ func (a *Adapter) EnsureAllReleasesExist() (controller.OperationResult, error) {
return controller.ContinueProcessing()
}

// EnsureOverrideSnapshotValid is an operation that ensure the manually created override snapshot is valid,
// mark it as valid if it is
func (a *Adapter) EnsureOverrideSnapshotValid() (controller.OperationResult, error) {
if !gitops.IsOverrideSnapshot(a.snapshot) {
a.logger.Info("The Snapshot was not override snapshot, skipping")
return controller.ContinueProcessing()
}

var err error
if !controllerutil.HasControllerReference(a.snapshot) {
a.snapshot, err = gitops.SetOwnerReference(a.context, a.client, a.snapshot, a.application)
if err != nil {
a.logger.Error(err, fmt.Sprintf("Failed to set owner reference for snapshot %s/%s", a.snapshot.Namespace, a.snapshot.Name))
return controller.RequeueWithError(err)
}
a.logger.Info("Application has been marked as snapshot owner",
"snapshot.Namespace", a.snapshot.Namespace, "snapshot.Name", a.snapshot.Name)
}

// validate all components' containerImages/source in snapshot, make all errors joined
var errsForSnapshot error
for _, snapshotComponent := range a.snapshot.Spec.Components {
snapshotComponent := snapshotComponent //G601
_, err := a.loader.GetComponent(a.context, a.client, snapshotComponent.Name, a.snapshot.Namespace)
if err != nil {
a.logger.Error(err, "Failed to get component from applicaion, won't update global candidate list for this component", "application.Name", a.application.Name, "component.Name", snapshotComponent.Name)
_, loaderError := h.HandleLoaderError(a.logger, err, snapshotComponent.Name, a.application.Name)
if loaderError != nil {
return controller.RequeueWithError(loaderError)
} else {
errsForSnapshot = errors.Join(errsForSnapshot, fmt.Errorf("snapshotComponent %s defined in snapshot %s doesn't exist under application %s/%s", snapshotComponent.Name, a.snapshot.Name, a.application.Namespace, a.application.Name))
}
continue
}

err = gitops.ValidateImageDigest(snapshotComponent.ContainerImage)
if err != nil {
a.logger.Error(err, "containerImage in snapshotComponent has no valid digest", "snapshotComponent.Name", snapshotComponent.Name, "snapshotComponent.ContainerImage", snapshotComponent.ContainerImage)
errsForSnapshot = errors.Join(errsForSnapshot, err)
continue
}

if !gitops.IsGitSourceValid(snapshotComponent) {
a.logger.Error(err, "snapshotComponent has no git url/revision", "snapshotComponent.Name", snapshotComponent.Name, "snapshotComponent.ContainerImage", snapshotComponent.ContainerImage)
errsForSnapshot = errors.Join(errsForSnapshot, err)
continue
}
}

if errsForSnapshot != nil {
a.logger.Error(errsForSnapshot, "mark the override snapshot as invalid due to invalid containerImage or git source")
if gitops.IsSnapshotMarkedAsInvalid(a.snapshot) {
err = gitops.MarkSnapshotAsInvalid(a.context, a.client, a.snapshot, errsForSnapshot.Error())
if err != nil {
a.logger.Error(err, "Failed to update snapshot to Invalid",
"snapshot.Namespace", a.snapshot.Namespace, "snapshot.Name", a.snapshot.Name)
return controller.RequeueWithError(err)
}
}
a.logger.Info("Snapshot has been marked as invalid",
"snapshot.Namespace", a.snapshot.Namespace, "snapshot.Name", a.snapshot.Name)
}
return controller.ContinueProcessing()
}

// createMissingReleasesForReleasePlans checks if there's existing Releases for a given list of ReleasePlans and creates
// new ones if they are missing. In case the Releases can't be created, an error will be returned.
func (a *Adapter) createMissingReleasesForReleasePlans(application *applicationapiv1alpha1.Application, releasePlans *[]releasev1alpha1.ReleasePlan, snapshot *applicationapiv1alpha1.Snapshot) error {
Expand Down
78 changes: 78 additions & 0 deletions internal/controller/snapshot/snapshot_adapter_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import (
"time"

"github.com/tonglil/buflogr"
"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"

. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
Expand Down Expand Up @@ -62,6 +63,7 @@ var _ = Describe("Snapshot Adapter", Ordered, func() {
hasSnapshotPR *applicationapiv1alpha1.Snapshot
hasOverRideSnapshot *applicationapiv1alpha1.Snapshot
hasInvalidSnapshot *applicationapiv1alpha1.Snapshot
hasInvalidOverrideSnapshot *applicationapiv1alpha1.Snapshot
integrationTestScenario *v1beta2.IntegrationTestScenario
integrationTestScenarioForInvalidSnapshot *v1beta2.IntegrationTestScenario
env *applicationapiv1alpha1.Environment
Expand Down Expand Up @@ -281,6 +283,45 @@ var _ = Describe("Snapshot Adapter", Ordered, func() {
}
Expect(k8sClient.Create(ctx, hasOverRideSnapshot)).Should(Succeed())

hasInvalidOverrideSnapshot = &applicationapiv1alpha1.Snapshot{
ObjectMeta: metav1.ObjectMeta{
Name: "snapshotpr-sample-override-invalid",
Namespace: "default",
Labels: map[string]string{
gitops.SnapshotTypeLabel: gitops.SnapshotOverrideType,
},
},
Spec: applicationapiv1alpha1.SnapshotSpec{
Application: hasApp.Name,
Components: []applicationapiv1alpha1.SnapshotComponent{
{
Name: hasComp.Name,
ContainerImage: sample_image + "@" + sampleDigest,
Source: applicationapiv1alpha1.ComponentSource{
ComponentSourceUnion: applicationapiv1alpha1.ComponentSourceUnion{
GitSource: &applicationapiv1alpha1.GitSource{
Revision: sample_revision,
},
},
},
},
{
Name: hasCompMissingImageDigest.Name,
ContainerImage: sample_image,
Source: applicationapiv1alpha1.ComponentSource{
ComponentSourceUnion: applicationapiv1alpha1.ComponentSourceUnion{
GitSource: &applicationapiv1alpha1.GitSource{
URL: SampleRepoLink,
Revision: sample_revision,
},
},
},
},
},
},
}
Expect(k8sClient.Create(ctx, hasInvalidOverrideSnapshot)).Should(Succeed())

Eventually(func() error {
err := k8sClient.Get(ctx, types.NamespacedName{
Name: hasSnapshot.Name,
Expand All @@ -297,6 +338,8 @@ var _ = Describe("Snapshot Adapter", Ordered, func() {
Expect(err == nil || errors.IsNotFound(err)).To(BeTrue())
err = k8sClient.Delete(ctx, hasOverRideSnapshot)
Expect(err == nil || errors.IsNotFound(err)).To(BeTrue())
err = k8sClient.Delete(ctx, hasInvalidOverrideSnapshot)
Expect(err == nil || errors.IsNotFound(err)).To(BeTrue())
})

AfterAll(func() {
Expand Down Expand Up @@ -1141,6 +1184,41 @@ var _ = Describe("Snapshot Adapter", Ordered, func() {
})
})

When("Adapter is created for override snapshot", func() {
var buf bytes.Buffer

It("ensures override snapshot with invalid snapshotComponent is marked as invalid", func() {
log := helpers.IntegrationLogger{Logger: buflogr.NewWithBuffer(&buf)}
Expect(gitops.IsSnapshotValid(hasInvalidOverrideSnapshot)).To(BeTrue())
Expect(controllerutil.HasControllerReference(hasInvalidOverrideSnapshot)).To(BeFalse())
adapter = NewAdapter(ctx, hasInvalidOverrideSnapshot, hasApp, log, loader.NewMockLoader(), k8sClient)
adapter.context = toolkit.GetMockedContext(ctx, []toolkit.MockData{
{
ContextKey: loader.ApplicationContextKey,
Resource: hasApp,
},
{
ContextKey: loader.ComponentContextKey,
Resource: hasComp,
},
{
ContextKey: loader.SnapshotContextKey,
Resource: hasInvalidOverrideSnapshot,
},
{
ContextKey: loader.SnapshotComponentsContextKey,
Resource: []applicationapiv1alpha1.Component{*hasComp, *hasCompMissingImageDigest},
},
})

result, err := adapter.EnsureOverrideSnapshotValid()
Expect(result.CancelRequest).To(BeFalse())
Expect(result.RequeueRequest).To(BeFalse())
Expect(controllerutil.HasControllerReference(hasInvalidOverrideSnapshot)).To(BeTrue())
Expect(buf.String()).Should(ContainSubstring("Snapshot has been marked as invalid"))
Expect(err).ToNot(HaveOccurred())
})
})
})

func getAllIntegrationPipelineRunsForSnapshot(ctx context.Context, snapshot *applicationapiv1alpha1.Snapshot) ([]tektonv1.PipelineRun, error) {
Expand Down
2 changes: 2 additions & 0 deletions internal/controller/snapshot/snapshot_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu
adapter.EnsureGlobalCandidateImageUpdated,
adapter.EnsureRerunPipelineRunsExist,
adapter.EnsureIntegrationPipelineRunsExist,
adapter.EnsureRerunPipelineRunsExist,
})
}

Expand All @@ -130,6 +131,7 @@ type AdapterInterface interface {
EnsureRerunPipelineRunsExist() (controller.OperationResult, error)
EnsureIntegrationPipelineRunsExist() (controller.OperationResult, error)
EnsureGlobalCandidateImageUpdated() (controller.OperationResult, error)
EnsureOverrideSnapshotValid() (controller.OperationResult, error)
}

// SetupController creates a new Integration controller and adds it to the Manager.
Expand Down

0 comments on commit 1d8a6a3

Please sign in to comment.