Skip to content

Feature/concert SBOM integration#283

Merged
dennis-behm merged 15 commits into
IBM:mainfrom
suman-gopinath:feature/concert-sbom-integration
Nov 7, 2024
Merged

Feature/concert SBOM integration#283
dennis-behm merged 15 commits into
IBM:mainfrom
suman-gopinath:feature/concert-sbom-integration

Conversation

@suman-gopinath
Copy link
Copy Markdown
Contributor

This pull request is to implement a feature integrating z/OS builds with IBM Concert This includes changes within the packaging script to create concert specific SBOM manifest or configurations. IBM Concert requires zOS Application builds to generate build config in the format https://github.ibm.com/roja/sample_app_data/blob/main/102/system-z/build-config-sample.yaml so that it can be supplied to concert dashboards. This enables zOS application changes to be visible in Concert dashboards along with the rest of the enterprise applications.

Suman and others added 4 commits October 24, 2024 13:27
Signed-off-by: Mathieu Dalbin <mathieu.dalbin@fr.ibm.com>
Signed-off-by: Mathieu Dalbin <mathieu.dalbin@fr.ibm.com>
Copy link
Copy Markdown
Member

@dennis-behm dennis-behm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello @suman-gopinath. thanks for the PR to enhance the PackageBuildOutputs script to enable for IBM Concert . I have a couple of questions about it. Please see below.

Please also update the README.md with this new capability.

if (props.generateSBOM && props.generateSBOM.toBoolean() && rc == 0) {
concertManifestGeneratorUtilities.addSBOMInfoToBuild(concertBuild, sbomFileName, sbomSerialNumber)
}
concertManifestGeneratorUtilities.writeBuildManifest(new File("$tempLoadDir/concert_build_manifest.yml"), props.fileEncoding, props.verbose)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The tar file is already created at this point. Should concert_build_manifest.yml be part of the package?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since the manifest needs the name of the tar. I suggest we leave it out this time

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Never mind. Added it now

@Field Properties props = null
def scriptDir = new File(getClass().protectionDomain.codeSource.location.path).parent
@Field def wdManifestGeneratorUtilities = loadScript(new File("${scriptDir}/utilities/WaziDeployManifestGenerator.groovy"))
@Field def concertManifestGeneratorUtilities = loadScript(new File("${scriptDir}/utilities/concertBuildManifestGenerator.groovy"))
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Like for the sbomUtilities, we are only loading the utilities when the user has turned it on. Can we apply the same approach for the concertManifestGeneratorUtilities?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed

if (!props.publish) {
println("*! [ERROR] Missing publish parameter ('--publish'). It is required for generating the Concert Build Manifest file.")
rc = 2
}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it a correct assumption that generating the concert build manifest file, will only work when only one BuildReport.json is passed to the PackageBuildOutputsScript? If so, we should validate that in this block.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We will need to discuss this

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done. Added the check. Need some more tests

# Default: false
generateWaziDeployAppManifest=false

# Boolean setting to define if the Wazi Deploy Application Manifest file should be generated
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
# Boolean setting to define if the Wazi Deploy Application Manifest file should be generated
# Boolean setting to define if the IBM Concert Build manifest file should be generated

generateWaziDeployAppManifest=false

# Boolean setting to define if the Wazi Deploy Application Manifest file should be generated
# Please note that the cli option `generateWaziDeployAppManifest` can override this setting and activate it.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
# Please note that the cli option `generateWaziDeployAppManifest` can override this setting and activate it.
# Please note that the cli option `generateConcertBuildManifest` can override this setting and activate it.

if (props.generateSBOM && props.generateSBOM.toBoolean()) {
sbomUtilities = loadScript(new File("${scriptDir}/utilities/sbomGenerator.groovy"))
sbomSerialNumber = "url:uuid:" + UUID.randomUUID().toString()
sbomFileName = "${buildNumber}_sbom.json"
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the file name format rather something that we should declare at the beginning of the script?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like the buildNumber is retrieved from the build report. So, it might make sense to retain this where it is.

Copy link
Copy Markdown
Member

@M-DLB M-DLB left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some comments on the README file. Thank you @suman-gopinath !

Comment thread Pipeline/PackageBuildOutputs/README.md Outdated
Comment thread Pipeline/PackageBuildOutputs/README.md Outdated
Comment thread Pipeline/PackageBuildOutputs/README.md Outdated
Comment thread Pipeline/PackageBuildOutputs/README.md Outdated
Comment thread Pipeline/PackageBuildOutputs/README.md Outdated
suman-gopinath and others added 5 commits November 6, 2024 10:44
Co-authored-by: Mathieu Dalbin <mathieu.dalbin@fr.ibm.com>
Co-authored-by: Mathieu Dalbin <mathieu.dalbin@fr.ibm.com>
Co-authored-by: Mathieu Dalbin <mathieu.dalbin@fr.ibm.com>
Co-authored-by: Mathieu Dalbin <mathieu.dalbin@fr.ibm.com>
Co-authored-by: Mathieu Dalbin <mathieu.dalbin@fr.ibm.com>
@suman-gopinath
Copy link
Copy Markdown
Contributor Author

Accepted all of the README suggestions. I am good with it

Copy link
Copy Markdown
Member

@dennis-behm dennis-behm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@suman-gopinath thanks for addressing all comments, thanks for the contribution

@dennis-behm dennis-behm merged commit c1a366b into IBM:main Nov 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants