GHA Artifacts: Update to v4

[Maffooch]

With the upcoming deprecations of version 3 of actions/upload-artifact and actions/download-artifact, we must get to v4 by following the migration guide

[sc-8285]

[dryrunsecurity]

DryRun Security Summary

The pull request focuses on updating the GitHub Actions workflows used for testing and deployment of the DefectDojo application, including upgrading action versions, improving artifact management, updating Kubernetes deployment, and changes to the release drafting workflow, with a focus on maintaining security considerations throughout the changes.

Expand for full summary

Summary:

The code changes in this pull request are primarily focused on updates to the GitHub Actions workflows used for various testing and deployment tasks related to the DefectDojo application. The changes include:

1. Upgrading the versions of the actions/download-artifact and actions/upload-artifact GitHub Actions used in the workflows.
2. Improvements to the artifact management and loading processes, such as downloading and merging multiple artifacts and updating the Docker image load paths.
3. Updates to the Kubernetes deployment workflow, including downloading and loading Docker images from artifacts, configuring HELM repositories, and deploying the DefectDojo application.
4. Changes to the release drafting workflow, which updates the release notes and uploads the OpenAPI Specification (OAS) files as release assets.

From an application security perspective, the changes do not introduce any obvious security vulnerabilities. However, it's important to maintain a vigilant approach and consider the following security aspects:

1. Ensure the integrity and security of the Docker images being used, including regular vulnerability scanning and secure configuration.
2. Review the Kubernetes deployment configuration to identify any potential security misconfigurations or exposures.
3. Verify the security of the OAS files being uploaded as release assets and maintain appropriate access controls.
4. Monitor the versions of the GitHub Actions used in the workflows and keep them up-to-date to address any known security vulnerabilities.

Overall, the changes in this pull request appear to be focused on improving the reliability and maintainability of the testing and deployment workflows, which can have a positive impact on the application's security posture. However, it's crucial to maintain a comprehensive security review process and address any potential security concerns that may arise.

Files Changed:

1. .github/workflows/integration-tests.yml: This workflow has been updated to use the latest version of the actions/download-artifact action, which allows downloading and merging multiple artifacts. The changes are focused on improving the reliability and maintainability of the integration testing process.

2. .github/workflows/build-docker-images-for-testing.yml: The changes in this file include upgrading the actions/upload-artifact action to version 4 and renaming the artifact upload for better organization. The workflow is responsible for building Docker images for testing purposes, so it's important to ensure the security of the Dockerfiles and the built images.

3. .github/workflows/fetch-oas.yml: This workflow has been updated to use the latest version of the actions/upload-artifact action. The changes also include setting the artifact retention period to 1 day, which can help minimize the potential attack surface.

4. .github/workflows/k8s-tests.yml: The changes in this file update the actions/download-artifact action to version 4 and implement the process of downloading and loading Docker images from artifacts into the Minikube environment. The Kubernetes deployment configuration should be reviewed for any potential security concerns.

5. .github/workflows/release-drafter.yml: The only change in this file is the update to the actions/download-artifact action version. This workflow is responsible for creating and updating GitHub releases, so it's important to ensure the integrity and security of the release assets.

6. .github/workflows/rest-framework-tests.yml: The changes in this file update the actions/download-artifact action to version 4 and improve the process of downloading and loading Docker images for the REST framework unit tests.

Code Analysis

We ran 9 analyzers against 6 files and 0 analyzers had findings. 9 analyzers had no findings.

Riskiness

🟢 Risk threshold not exceeded.

View PR in the DryRun Dashboard.

[mtesauro]

Approved

[cneill]

I'm not entirely sure of the behavior of merge-multiple based on the migration documentation, but unless I'm reading something wrong it seems like we need to add the built-docker-image- prefix to the images we try to download+load into Docker

I also wonder if we need to specify the overwrite key on upload since we're not including something like the workflow run ID/etc in the artifact name.

[cneill]

Will this actually trim the built-docker-image- prefix from these image names, or do we need something like this instead? Wasn't clear to me from the migration guide...

Suggested change

	docker load -i built-docker-image/nginx-${{ matrix.os }}_img
	docker load -i built-docker-image/django-${{ matrix.os }}_img
	docker load -i built-docker-image/integration-tests-debian_img
	docker load -i built-docker-image/built-docker-image-nginx-${{ matrix.os }}_img
	docker load -i built-docker-image/built-docker-image-django-${{ matrix.os }}_img
	docker load -i built-docker-image/built-docker-image-integration-tests-debian_img

[Maffooch]

I don't believe the prefix is trimmed. Do we need the prefix to be trimmed?

[Maffooch]

Also I didn't realize when I left the last comment, but the names of the images themselves do not have a built-docker-image- prefix. The name of the image stored is captured in the path entry:

path: ${{ matrix.docker-image }}-${{ matrix.os }}_img

This value is unchanged in this PR

[cneill]

Ahh, I think I mixed up the with (artifact name) and the path

[cneill]

Same question as above..

Suggested change

	docker load -i built-docker-image/nginx-${{ matrix.os }}_img
	docker load -i built-docker-image/django-${{ matrix.os }}_img
	docker load -i built-docker-image/built-docker-image-nginx-${{ matrix.os }}_img
	docker load -i built-docker-image/built-docker-image-django-${{ matrix.os }}_img

[cneill]

Same as above..

Suggested change

	docker load -i built-docker-image/nginx-${{ matrix.os }}_img
	docker load -i built-docker-image/django-${{ matrix.os }}_img
	docker load -i built-docker-image/built-docker-image-nginx-${{ matrix.os }}_img
	docker load -i built-docker-image/built-docker-image-django-${{ matrix.os }}_img

[Maffooch]

I'm not entirely sure of the behavior of merge-multiple based on the migration documentation, but unless I'm reading something wrong it seems like we need to add the built-docker-image- prefix to the images we try to download+load into Docker

Why would we need the prefix? To make the names of the images more descriptive?

I also wonder if we need to specify the overwrite key on upload since we're not including something like the workflow run ID/etc in the artifact name.

I am not sure about this one. With the upcoming brownouts next week, I think it would be preferable to get this merged in, and then investigate how the overwrite flag will add value

Also please keep in mind, if the docker images are able to be downloaded so that the unit tests can run, then this implementation is functioning as expected. Please see previous failed implementations in the commit history to see what all went into this 😄

[Maffooch]

@cneill we can discuss how this can be iterated on after the release today 😄