[DEVHAS-492] Test for Component Devfile with Private Parent references (

#426)

.gitignore

@@ -12,4 +12,5 @@ tmp
 output.log
 syncer.yaml
 .DS_Store
-pact.log
+pact.log
+*resource.file*

cdq-analysis/go.mod

@@ -5,7 +5,7 @@ go 1.19
 require (
 	github.com/devfile/alizer v1.2.2-0.20231004141146-f36141673c7f
 	github.com/devfile/api/v2 v2.2.1
-	github.com/devfile/library/v2 v2.2.2-0.20231130202909-20a0c910b0e6
+	github.com/devfile/library/v2 v2.2.2-0.20231206202302-705f00dd96f5
 	github.com/devfile/registry-support/index/generator v0.0.0-20221018203505-df96d34d4273
 	github.com/devfile/registry-support/registry-library v0.0.0-20221018213054-47b3ffaeadba
 	github.com/go-logr/logr v1.2.4

cdq-analysis/go.sum

@@ -311,8 +311,8 @@ github.com/devfile/api/v2 v2.2.1/go.mod h1:qp8jcw12y1JdCsxjK/7LJ7uWaJOxcY1s2LUk5
 github.com/devfile/library v1.2.1-0.20211104222135-49d635cb492f/go.mod h1:uFZZdTuRqA68FVe/JoJHP92CgINyQkyWnM2Qyiim+50=
 github.com/devfile/library v1.2.1-0.20220308191614-f0f7e11b17de/go.mod h1:GSPfJaBg0+bBjBHbwBE5aerJLH6tWGQu2q2rHYd9czM=
 github.com/devfile/library/v2 v2.0.1/go.mod h1:paJ0PARAVy0br13VpBEQ4fO3rZVDxWtooQ29+23PNBk=
-github.com/devfile/library/v2 v2.2.2-0.20231130202909-20a0c910b0e6 h1:4WisxE/COs2ITGxZ1yA7qs+3HSFni4wQd4ZAiaOiBOY=
-github.com/devfile/library/v2 v2.2.2-0.20231130202909-20a0c910b0e6/go.mod h1:zKKhnbSLXi8vu46c5RLr+y4kY9S9Ubi0SeCm3awndsw=
+github.com/devfile/library/v2 v2.2.2-0.20231206202302-705f00dd96f5 h1:RcqH+Kg4mYWXRYyjNC+tQ3rZmsHEiAjXd1mCZfaoc8Y=
+github.com/devfile/library/v2 v2.2.2-0.20231206202302-705f00dd96f5/go.mod h1:zKKhnbSLXi8vu46c5RLr+y4kY9S9Ubi0SeCm3awndsw=
 github.com/devfile/registry-support/index/generator v0.0.0-20220222194908-7a90a4214f3e/go.mod h1:iRPBxs+ZjfLEduVXpCCIOzdD2588Zv9OCs/CcXMcCCY=
 github.com/devfile/registry-support/index/generator v0.0.0-20220527155645-8328a8a883be/go.mod h1:1fyDJL+fPHtcrYA6yjSVWeLmXmjCNth0d5Rq1rvtryc=
 github.com/devfile/registry-support/index/generator v0.0.0-20221018203505-df96d34d4273 h1:DXENQSRTEDsk9com38njPg5511DD12HPIgzyFUErnpM=

cdq-analysis/pkg/devfile_test.go

@@ -345,7 +345,8 @@ func TestValidateDevfile(t *testing.T) {
 		ConvertKubernetesContentInUri: &convert,
 	}
 
-	parserArgs.DevfileUtilsClient = devfileParserUtil.NewMockDevfileUtilsClient()
+	mockDevfileUtilsClient := devfileParserUtil.NewMockDevfileUtilsClient()
+	parserArgs.DevfileUtilsClient = &mockDevfileUtilsClient
 
 	springDevfileParser := parserArgs
 	springDevfileParser.URL = "https://raw.githubusercontent.com/devfile-samples/devfile-sample-java-springboot-basic/main/devfile.yaml"

controllers/component_controller.go

@@ -224,8 +224,11 @@ func (r *ComponentReconciler) Reconcile(ctx context.Context, req ctrl.Request) (
 	for _, condition := range component.Status.Conditions {
 		if forceGenerateGitopsResource || (condition.Type == "GitOpsResourcesGenerated" && condition.Reason == "GenerateError" && condition.Status == metav1.ConditionFalse) {
 			log.Info(fmt.Sprintf("Re-attempting GitOps generation for %s", component.Name))
-			// Parse the Component Devfile
-			compDevfileData, err := cdqanalysis.ParseDevfileWithParserArgs(&devfileParser.ParserArgs{Data: []byte(component.Status.Devfile), Token: gitToken})
+			// Parse the Component CR Devfile
+			// Not necessary to pass in a Token or DevfileUtils client to the parser here since the devfileBytes has:
+			// 1. Already been flattened on the create reconcile, so private parents are already expanded
+			// 2. Kubernetes Component Uri has already been converted to inlined content with a Token if required by default on the first parse
+			compDevfileData, err := cdqanalysis.ParseDevfileWithParserArgs(&devfileParser.ParserArgs{Data: []byte(component.Status.Devfile)})
 			if err != nil {
 				errMsg := fmt.Sprintf("Unable to parse the devfile from Component status and re-attempt GitOps generation, exiting reconcile loop %v", req.NamespacedName)
 				log.Error(err, errMsg)
@@ -401,8 +404,12 @@ func (r *ComponentReconciler) Reconcile(ctx context.Context, req ctrl.Request) (
 		}
 
 		if devfileLocation != "" {
-			// Parse the Component Devfile
 			log.Info(fmt.Sprintf("Parsing Devfile from the Devfile location %s... %v", devfileLocation, req.NamespacedName))
+			// Parse the Component CR Devfile
+			// Pass in a Token and a DevfileUtils client because we need to
+			// 1. Flatten the Devfile and access a private parent if necessary
+			// 2. Convert the Kubernetes Uri to Inline by default
+			// 3. Provide a way to mock output for Component controller tests
 			compDevfileData, err = cdqanalysis.ParseDevfileWithParserArgs(&devfileParser.ParserArgs{URL: devfileLocation, Token: gitToken, DevfileUtilsClient: r.DevfileUtilsClient})
 
 			if err != nil {
@@ -411,9 +418,12 @@ func (r *ComponentReconciler) Reconcile(ctx context.Context, req ctrl.Request) (
 				return ctrl.Result{}, err
 			}
 		} else {
-			// Parse the Component Devfile
 			log.Info(fmt.Sprintf("Parsing Devfile from the Devfile bytes %v... %v", len(devfileBytes), req.NamespacedName))
-			compDevfileData, err = cdqanalysis.ParseDevfileWithParserArgs(&devfileParser.ParserArgs{Data: devfileBytes, Token: gitToken})
+			// Parse the Component CR Devfile
+			// Not necessary to pass in a Token or a DevfileUtils client to the parser here on devfileBytes, since:
+			// 1. devfileBytes are only used from a DockerfileURL or an Image, Component CR source (check if conditions above on Component CR sources)
+			// 2. We dont access any resources for either of these cases in devfile/library
+			compDevfileData, err = cdqanalysis.ParseDevfileWithParserArgs(&devfileParser.ParserArgs{Data: devfileBytes})
 
 			if err != nil {
 				log.Error(err, fmt.Sprintf("Unable to parse the devfile from Component, exiting reconcile loop %v", req.NamespacedName))
@@ -430,8 +440,11 @@ func (r *ComponentReconciler) Reconcile(ctx context.Context, req ctrl.Request) (
 		}
 
 		if hasApplication.Status.Devfile != "" {
-			// Get the devfile of the hasApp CR
-			hasAppDevfileData, err := cdqanalysis.ParseDevfileWithParserArgs(&devfileParser.ParserArgs{Data: []byte(hasApplication.Status.Devfile), Token: gitToken})
+			// Parse the Application CR Devfile
+			// No need to invoke devfile/library parser with a Token or a DevfileUtils client because the Application CR Devfile model:
+			// 1. Is constructed in the Application controller and there is no need for a Token
+			// 2. Only consists of Devfile metadata attributes and projects to store the Component CR information
+			hasAppDevfileData, err := cdqanalysis.ParseDevfileWithParserArgs(&devfileParser.ParserArgs{Data: []byte(hasApplication.Status.Devfile)})
 
 			if err != nil {
 				log.Error(err, fmt.Sprintf("Unable to parse the devfile from Application, exiting reconcile loop %v", req.NamespacedName))
@@ -485,8 +498,11 @@ func (r *ComponentReconciler) Reconcile(ctx context.Context, req ctrl.Request) (
 		// If the model already exists, see if fields have been updated
 		log.Info(fmt.Sprintf("Checking if the Component has been updated %v", req.NamespacedName))
 
-		// Parse the Component Devfile
-		hasCompDevfileData, err := cdqanalysis.ParseDevfileWithParserArgs(&devfileParser.ParserArgs{Data: []byte(component.Status.Devfile), Token: gitToken})
+		// Parse the Component CR Devfile
+		// Not necessary to pass in a Token or DevfileUtils client to the parser here since the devfileBytes has:
+		// 1. Already been flattened on the create reconcile, so private parents are already expanded
+		// 2. Kubernetes Component Uri has already been converted to inlined content with a Token if required by default on the first parse
+		hasCompDevfileData, err := cdqanalysis.ParseDevfileWithParserArgs(&devfileParser.ParserArgs{Data: []byte(component.Status.Devfile)})
 
 		if err != nil {
 			log.Error(err, fmt.Sprintf("Unable to parse the devfile from Component status, exiting reconcile loop %v", req.NamespacedName))
@@ -501,8 +517,11 @@ func (r *ComponentReconciler) Reconcile(ctx context.Context, req ctrl.Request) (
 			return ctrl.Result{}, err
 		}
 
-		// Read the devfile again to compare it with any updates
-		oldCompDevfileData, err := cdqanalysis.ParseDevfileWithParserArgs(&devfileParser.ParserArgs{Data: []byte(component.Status.Devfile), Token: gitToken})
+		// Parse the Component CR Devfile again to compare it with any updates
+		// Not necessary to pass in a Token or DevfileUtils client to the parser here since the devfileBytes has:
+		// 1. Already been flattened on the create reconcile, so private parents are already expanded
+		// 2. Kubernetes Component Uri has already been converted to inlined content with a Token if required by default on the first parse
+		oldCompDevfileData, err := cdqanalysis.ParseDevfileWithParserArgs(&devfileParser.ParserArgs{Data: []byte(component.Status.Devfile)})
 
 		if err != nil {
 			log.Error(err, fmt.Sprintf("Unable to parse the devfile from Component status, exiting reconcile loop %v", req.NamespacedName))

controllers/component_controller_test.go

@@ -2411,6 +2411,158 @@ var _ = Describe("Component controller", func() {
 			// Delete the specified HASApp resource
 			deleteHASAppCR(hasAppLookupKey)
 		})
+
+		Context("Create Private Component with basic field set and a private parent uri", func() {
+			It("Should create successfully and update the Application", func() {
+				ctx := context.Background()
+
+				applicationName := HASAppName + "26"
+				componentName := HASCompName + "26"
+
+				originalPort := 1111
+				updatedPort := 2222
+
+				// Create a git secret
+				tokenSecret := &corev1.Secret{
+					TypeMeta: metav1.TypeMeta{
+						Kind: "Secret",
+					},
+					ObjectMeta: metav1.ObjectMeta{
+						Name:      componentName,
+						Namespace: HASAppNamespace,
+					},
+					StringData: map[string]string{
+						"password": "parent-devfile", // notsecret - see mock implementation in devfile/library https://github.com/devfile/library/blob/main/pkg/util/mock.go
+					},
+				}
+
+				Expect(k8sClient.Create(ctx, tokenSecret)).Should(Succeed())
+
+				createAndFetchSimpleApp(applicationName, HASAppNamespace, DisplayName, Description)
+
+				hasComp := &appstudiov1alpha1.Component{
+					TypeMeta: metav1.TypeMeta{
+						APIVersion: "appstudio.redhat.com/v1alpha1",
+						Kind:       "Component",
+					},
+					ObjectMeta: metav1.ObjectMeta{
+						Name:      componentName,
+						Namespace: HASAppNamespace,
+					},
+					Spec: appstudiov1alpha1.ComponentSpec{
+						ComponentName: ComponentName,
+						Application:   applicationName,
+						Secret:        componentName,
+						Source: appstudiov1alpha1.ComponentSource{
+							ComponentSourceUnion: appstudiov1alpha1.ComponentSourceUnion{
+								GitSource: &appstudiov1alpha1.GitSource{
+									URL: "https://github.com/maysunfaisal/devfile-sample-python-basic-private", // It doesn't matter if we are using pub/pvt repo here. We are mock testing the token, "parent-devfile" returns a mock devfile and mock parent. See https://github.com/devfile/library/blob/main/pkg/util/mock.go
+								},
+							},
+						},
+						TargetPort: originalPort,
+					},
+				}
+				Expect(k8sClient.Create(ctx, hasComp)).Should(Succeed())
+
+				// Look up the has app resource that was created.
+				// num(conditions) may still be < 2 (GeneratedGitOps, Created) on the first try, so retry until at least _some_ condition is set
+				hasCompLookupKey := types.NamespacedName{Name: componentName, Namespace: HASAppNamespace}
+				createdHasComp := &appstudiov1alpha1.Component{}
+				Eventually(func() bool {
+					k8sClient.Get(context.Background(), hasCompLookupKey, createdHasComp)
+					return len(createdHasComp.Status.Conditions) > 1 && createdHasComp.Status.GitOps.RepositoryURL != ""
+				}, timeout, interval).Should(BeTrue())
+
+				// Verify that the GitOpsGenerated status condition was also set
+				// ToDo: Add helper func for accessing the status conditions in a better way
+				gitopsCondition := createdHasComp.Status.Conditions[len(createdHasComp.Status.Conditions)-2]
+				Expect(gitopsCondition.Type).To(Equal("GitOpsResourcesGenerated"))
+				Expect(gitopsCondition.Status).To(Equal(metav1.ConditionTrue))
+
+				// Make sure the devfile model was properly set in Component
+				Expect(createdHasComp.Status.Devfile).Should(Not(Equal("")))
+
+				hasAppLookupKey := types.NamespacedName{Name: applicationName, Namespace: HASAppNamespace}
+				createdHasApp := &appstudiov1alpha1.Application{}
+				Eventually(func() bool {
+					k8sClient.Get(context.Background(), hasAppLookupKey, createdHasApp)
+					return len(createdHasApp.Status.Conditions) > 0 && strings.Contains(createdHasApp.Status.Devfile, ComponentName)
+				}, timeout, interval).Should(BeTrue())
+
+				// Make sure the devfile model was properly set in Application
+				Expect(createdHasApp.Status.Devfile).Should(Not(Equal("")))
+
+				// Check the Component devfile
+				_, err := cdqanalysis.ParseDevfileWithParserArgs(&parser.ParserArgs{Data: []byte(createdHasComp.Status.Devfile)})
+				Expect(err).Should(Not(HaveOccurred()))
+
+				// Check the HAS Application devfile
+				hasAppDevfile, err := cdqanalysis.ParseDevfileWithParserArgs(&parser.ParserArgs{Data: []byte(createdHasApp.Status.Devfile)})
+
+				Expect(err).Should(Not(HaveOccurred()))
+
+				// gitOpsRepo and appModelRepo should both be set
+				Expect(string(hasAppDevfile.GetMetadata().Attributes["gitOpsRepository.url"].Raw)).Should(Not(Equal("")))
+				Expect(string(hasAppDevfile.GetMetadata().Attributes["appModelRepository.url"].Raw)).Should(Not(Equal("")))
+
+				// gitOpsRepo set in the component equal the repository in the app cr's devfile
+				gitopsRepo := hasAppDevfile.GetMetadata().Attributes.GetString("gitOpsRepository.url", &err)
+				Expect(err).Should(Not(HaveOccurred()))
+				Expect(string(createdHasComp.Status.GitOps.RepositoryURL)).Should(Equal(gitopsRepo))
+
+				// Commit ID should be set in the gitops repository and not be empty
+				Expect(createdHasComp.Status.GitOps.CommitID).Should(Not(BeEmpty()))
+
+				hasProjects, err := hasAppDevfile.GetProjects(common.DevfileOptions{})
+				Expect(err).Should(Not(HaveOccurred()))
+				Expect(len(hasProjects)).ShouldNot(Equal(0))
+
+				nameMatched := false
+				repoLinkMatched := false
+				for _, project := range hasProjects {
+					if project.Name == ComponentName {
+						nameMatched = true
+					}
+					if project.Git != nil && project.Git.GitLikeProjectSource.Remotes["origin"] == "https://github.com/maysunfaisal/devfile-sample-python-basic-private" {
+						repoLinkMatched = true
+					}
+				}
+				Expect(nameMatched).Should(Equal(true))
+				Expect(repoLinkMatched).Should(Equal(true))
+
+				// Update Component
+				createdHasComp.Spec.TargetPort = updatedPort
+
+				Expect(k8sClient.Update(ctx, createdHasComp)).Should(Succeed())
+
+				updatedHasComp := &appstudiov1alpha1.Component{}
+				Eventually(func() bool {
+					k8sClient.Get(context.Background(), hasCompLookupKey, updatedHasComp)
+					return updatedHasComp.Status.Conditions[len(updatedHasComp.Status.Conditions)-1].Type == "Updated"
+				}, timeout, interval).Should(BeTrue())
+
+				// Make sure the devfile model was properly set in Component
+				Expect(updatedHasComp.Status.Devfile).Should(Not(Equal("")))
+
+				// Check the Component updated devfile
+				hasCompUpdatedDevfile, err := cdqanalysis.ParseDevfileWithParserArgs(&parser.ParserArgs{Data: []byte(updatedHasComp.Status.Devfile)})
+
+				Expect(err).Should(Not(HaveOccurred()))
+
+				checklist := updateChecklist{
+					port: updatedPort,
+				}
+
+				verifyHASComponentUpdates(hasCompUpdatedDevfile, checklist, nil)
+
+				// Delete the specified HASComp resource
+				deleteHASCompCR(hasCompLookupKey)
+
+				// Delete the specified HASApp resource
+				deleteHASAppCR(hasAppLookupKey)
+			})
+		})
 	})
 
 })

controllers/start_test_env.go

@@ -96,6 +96,7 @@ func SetupTestEnv() (client.Client, *envtest.Environment, context.Context, conte
 	gomega.Expect(err).ToNot(gomega.HaveOccurred())
 
 	mockGhTokenClient := github.MockGitHubTokenClient{}
+	mockDevfileUtilsClient := devfileParserUtil.NewMockDevfileUtilsClient()
 
 	// Retrieve the option to specify a cdq-analysis image
 	cdqAnalysisImage := os.Getenv("CDQ_ANALYSIS_IMAGE")
@@ -123,7 +124,7 @@ func SetupTestEnv() (client.Client, *envtest.Environment, context.Context, conte
 			K8sClient: k8sClient,
 		},
 		GitHubTokenClient:  mockGhTokenClient,
-		DevfileUtilsClient: devfileParserUtil.NewMockDevfileUtilsClient(),
+		DevfileUtilsClient: &mockDevfileUtilsClient,
 	}).SetupWithManager(ctx, k8sManager)
 	gomega.Expect(err).ToNot(gomega.HaveOccurred())
 

go.mod

@@ -5,7 +5,7 @@ go 1.19
 require (
 	github.com/brianvoe/gofakeit/v6 v6.9.0
 	github.com/devfile/api/v2 v2.2.1
-	github.com/devfile/library/v2 v2.2.2-0.20231130202909-20a0c910b0e6
+	github.com/devfile/library/v2 v2.2.2-0.20231206202302-705f00dd96f5
 	github.com/go-logr/logr v1.2.4
 	github.com/gofri/go-github-ratelimit v1.0.3-0.20230428184158-a500e14de53f
 	github.com/golang/mock v1.6.0

go.sum

@@ -484,8 +484,8 @@ github.com/devfile/api/v2 v2.2.1/go.mod h1:qp8jcw12y1JdCsxjK/7LJ7uWaJOxcY1s2LUk5
 github.com/devfile/library v1.2.1-0.20211104222135-49d635cb492f/go.mod h1:uFZZdTuRqA68FVe/JoJHP92CgINyQkyWnM2Qyiim+50=
 github.com/devfile/library v1.2.1-0.20220308191614-f0f7e11b17de/go.mod h1:GSPfJaBg0+bBjBHbwBE5aerJLH6tWGQu2q2rHYd9czM=
 github.com/devfile/library/v2 v2.0.1/go.mod h1:paJ0PARAVy0br13VpBEQ4fO3rZVDxWtooQ29+23PNBk=
-github.com/devfile/library/v2 v2.2.2-0.20231130202909-20a0c910b0e6 h1:4WisxE/COs2ITGxZ1yA7qs+3HSFni4wQd4ZAiaOiBOY=
-github.com/devfile/library/v2 v2.2.2-0.20231130202909-20a0c910b0e6/go.mod h1:zKKhnbSLXi8vu46c5RLr+y4kY9S9Ubi0SeCm3awndsw=
+github.com/devfile/library/v2 v2.2.2-0.20231206202302-705f00dd96f5 h1:RcqH+Kg4mYWXRYyjNC+tQ3rZmsHEiAjXd1mCZfaoc8Y=
+github.com/devfile/library/v2 v2.2.2-0.20231206202302-705f00dd96f5/go.mod h1:zKKhnbSLXi8vu46c5RLr+y4kY9S9Ubi0SeCm3awndsw=
 github.com/devfile/registry-support/index/generator v0.0.0-20220222194908-7a90a4214f3e/go.mod h1:iRPBxs+ZjfLEduVXpCCIOzdD2588Zv9OCs/CcXMcCCY=
 github.com/devfile/registry-support/index/generator v0.0.0-20220527155645-8328a8a883be/go.mod h1:1fyDJL+fPHtcrYA6yjSVWeLmXmjCNth0d5Rq1rvtryc=
 github.com/devfile/registry-support/index/generator v0.0.0-20221018203505-df96d34d4273 h1:DXENQSRTEDsk9com38njPg5511DD12HPIgzyFUErnpM=

pkg/devfile/devfile.go

@@ -74,6 +74,9 @@ func GetResourceFromDevfile(log logr.Logger, devfileData data.DevfileData, deplo
 				src := parser.YamlSrc{
 					Data: []byte(component.Kubernetes.Inlined),
 				}
+				// No need to pass in the Token or the DevfileUtils client because
+				// it is inlined data and the file has already been parsed by the devfile/library
+				// on the initial parse and stored as inlined content.
 				values, err := parser.ReadKubernetesYaml(src, nil, nil)
 				if err != nil {
 					return parser.KubernetesResources{}, err