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

Adding Gitgaurdian scan support #1122

Closed
wants to merge 4 commits into from
Closed

Adding Gitgaurdian scan support #1122

wants to merge 4 commits into from

Conversation

rsundriyal
Copy link
Contributor

Adding GitGaurdian Scan support to scan the CI pipeline for every commit.
Changing Jenkins pipeline to declarative from scripted pipeline.

@rsundriyal rsundriyal requested a review from val-ms December 21, 2023 18:08
[$class: 'StringParameterValue', name: 'FRAMEWORK_BRANCH', value: "${params.FRAMEWORK_BRANCH}"],
[$class: 'StringParameterValue', name: 'VERSION', value: "${params.VERSION}"],
[$class: 'StringParameterValue', name: 'SHARED_LIB_BRANCH', value: "${params.SHARED_LIB_BRANCH}"]
]
Copy link
Contributor

Choose a reason for hiding this comment

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

Some of the indentation here is pretty deep - I think 8 spaces instead of 4. I recommend reducing to 4 throughout, for consistency and legitibility.

Comment on lines +59 to +68
stage('GitGaurdian Scan') {
environment {
GITGUARDIAN_API_KEY = credentials('gitgaudian-ravi-token')
}
agent { label "docker" }
steps {
withDockerContainer(args: "-i --entrypoint=''", image: 'gitguardian/ggshield:latest') {
sh 'ggshield secret scan ci'
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder how long this takes. We may wish to run this in a parallel block.

Comment on lines +83 to +88
sh """# Move the clamav-documentation here.
cp -r ../../clamav_documentation/* .
# Clean-up
rm -rf ../../clamav_documentation
rm -rf .git .nojekyll CNAME Placeholder || true
"""
Copy link
Contributor

Choose a reason for hiding this comment

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

This commit loses some indentation here and in other script blocks that makes it easier to see the script content separate from the jenkins command. I would prefer to keep that.

E.g.

                    sh """# Move the clamav-documentation here.
                        cp -r ../../clamav_documentation/* .
                        # Clean-up
                        rm -rf ../../clamav_documentation
                        rm -rf .git .nojekyll CNAME Placeholder || true
                        """

or maybe

                    sh """
                        # Move the clamav-documentation here.
                        cp -r ../../clamav_documentation/* .
                        # Clean-up
                        rm -rf ../../clamav_documentation
                        rm -rf .git .nojekyll CNAME Placeholder || true
                    """

archiveArtifacts(artifacts: "clamav-${params.VERSION}*.tar.gz", onlyIfSuccessful: true)
stage('GitGaurdian Scan') {
environment {
GITGUARDIAN_API_KEY = credentials('gitgaudian-ravi-token')
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
GITGUARDIAN_API_KEY = credentials('gitgaudian-ravi-token')
GITGUARDIAN_API_KEY = credentials('gitgaudian-ravi-token')

We may need a generic account/team account for this token so it doesn't have to be a personal one.

Also, minor typo "gitgaudian" should be "gitgaurdian".

@rsundriyal rsundriyal closed this by deleting the head repository Apr 30, 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.

2 participants