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.

Sign up for GitHub

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

Enable cross-Application nudging #434

Merged
merged 1 commit into from
Jan 11, 2024
Merged

Enable cross-Application nudging #434

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
12 changes: 4 additions & 8 deletions controllers/webhooks/component_webhook.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ func (r *ComponentWebhook) ValidateCreate(ctx context.Context, obj runtime.Objec
}

if len(comp.Spec.BuildNudgesRef) != 0 {
err := r.validateBuildNudgesRefGraph(ctx, comp.Spec.BuildNudgesRef, comp.Namespace, comp.Name, comp.Spec.Application)
err := r.validateBuildNudgesRefGraph(ctx, comp.Spec.BuildNudgesRef, comp.Namespace, comp.Name)
if err != nil {
return err
}
Expand Down Expand Up @@ -161,7 +161,7 @@ func (r *ComponentWebhook) ValidateUpdate(ctx context.Context, oldObj, newObj ru
return fmt.Errorf(appstudiov1alpha1.GitSourceUpdateError, *(newComp.Spec.Source.GitSource))
}
if len(newComp.Spec.BuildNudgesRef) != 0 {
err := r.validateBuildNudgesRefGraph(ctx, newComp.Spec.BuildNudgesRef, newComp.Namespace, newComp.Name, newComp.Spec.Application)
err := r.validateBuildNudgesRefGraph(ctx, newComp.Spec.BuildNudgesRef, newComp.Namespace, newComp.Name)
if err != nil {
return err
}
Expand Down Expand Up @@ -225,7 +225,7 @@ func (r *ComponentWebhook) ValidateDelete(ctx context.Context, obj runtime.Objec

// validateBuildNudgesRefGraph returns an error if a cycle was found in the 'build-nudges-ref' dependency graph
// If no cycle is found, it returns nil
func (r *ComponentWebhook) validateBuildNudgesRefGraph(ctx context.Context, nudgedComponentNames []string, componentNamespace string, componentName string, componentApp string) error {
func (r *ComponentWebhook) validateBuildNudgesRefGraph(ctx context.Context, nudgedComponentNames []string, componentNamespace string, componentName string) error {
for _, nudgedComponentName := range nudgedComponentNames {
if nudgedComponentName == componentName {
return fmt.Errorf("cycle detected: component %s cannot reference itself, directly or indirectly, via build-nudges-ref", nudgedComponentName)
Expand All @@ -240,11 +240,7 @@ func (r *ComponentWebhook) validateBuildNudgesRefGraph(ctx context.Context, nudg
}
}

if nudgedComponent.Spec.Application != componentApp {
return fmt.Errorf("component %s cannot be added to spec.build-nudges-ref as it belongs to a different application", nudgedComponentName)
}

err = r.validateBuildNudgesRefGraph(ctx, nudgedComponent.Spec.BuildNudgesRef, componentNamespace, componentName, componentApp)
err = r.validateBuildNudgesRefGraph(ctx, nudgedComponent.Spec.BuildNudgesRef, componentNamespace, componentName)
if err != nil {
return err
}
Expand Down
32 changes: 0 additions & 32 deletions controllers/webhooks/component_webhook_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -208,32 +208,6 @@ var _ = Describe("Application validation webhook", func() {
err := k8sClient.Create(ctx, nudgedComp)
Expect(err).Should(Not(HaveOccurred()))

// comp in a different app
differentAppComp := &appstudiov1alpha1.Component{
TypeMeta: metav1.TypeMeta{
APIVersion: "appstudio.redhat.com/v1alpha1",
Kind: "Component",
},
ObjectMeta: metav1.ObjectMeta{
Namespace: HASAppNamespace,
Name: uniqueHASCompName + "-new-app",
},
Spec: appstudiov1alpha1.ComponentSpec{
ComponentName: uniqueHASCompName + "-new-app",
Application: "test-application2",
Source: appstudiov1alpha1.ComponentSource{
ComponentSourceUnion: appstudiov1alpha1.ComponentSourceUnion{
GitSource: &appstudiov1alpha1.GitSource{
URL: SampleRepoLink,
},
},
},
},
}

err = k8sClient.Create(ctx, differentAppComp)
Expect(err).Should(Not(HaveOccurred()))

// nudgingComp
nudgingComp := &appstudiov1alpha1.Component{
TypeMeta: metav1.TypeMeta{
Expand Down Expand Up @@ -286,12 +260,6 @@ var _ = Describe("Application validation webhook", func() {
Expect(len(buildNudgedByList)).To(Equal(1))
Expect(buildNudgedByList[0] == uniqueHASCompName)

// Now attempt to update the build-nudges-ref field to an invalid component (different app)
createdHasComp.Spec.BuildNudgesRef = []string{uniqueHASCompName + "-new-app"}
err = k8sClient.Update(ctx, createdHasComp)
Expect(err).Should((HaveOccurred()))
Expect(err.Error()).Should(ContainSubstring("belongs to a different application"))

// Delete the specified HASComp resource
deleteHASCompCR(hasCompLookupKey)
})
Expand Down
3 changes: 1 addition & 2 deletions controllers/webhooks/component_webhook_unit_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -494,7 +494,6 @@ func TestValidateBuildNudgesRefGraph(t *testing.T) {
name: "nudged component belongs to different app",
compName: "component-invalid-app",
webhook: compWebhook,
errStr: "component component4 cannot be added to spec.build-nudges-ref as it belongs to a different application",
},
{
name: "complex component relationship - some valid, some not valid (self referential)",
Expand All @@ -514,7 +513,7 @@ func TestValidateBuildNudgesRefGraph(t *testing.T) {
component := &appstudiov1alpha1.Component{}
fakeClient.Get(context.Background(), types.NamespacedName{Namespace: "default", Name: test.compName}, component)

err := test.webhook.validateBuildNudgesRefGraph(context.Background(), component.Spec.BuildNudgesRef, "default", test.compName, component.Spec.Application)
err := test.webhook.validateBuildNudgesRefGraph(context.Background(), component.Spec.BuildNudgesRef, "default", test.compName)
var errStr string
if err != nil {
errStr = err.Error()
Expand Down
Loading