Skip to content

Conversation

@afdesk
Copy link
Contributor

@afdesk afdesk commented Nov 5, 2025

Description

This PR adds a validation to ensure that a volume containing GCR credentials is present before proceeding.
If such a volume is already mounted, the logic prevents redundant re-mounting to avoid duplication and potential conflicts.

Related issues

Checklist

  • I've read the guidelines for contributing to this repository.
  • I've added tests that prove my fix is effective or that my feature works.
  • I've updated the documentation with the relevant information (if needed).
  • I've added usage information (if the PR introduces new options)
  • I've included a "before" and "after" example to the description (if the PR is a user interface change).

@afdesk afdesk added this to the v0.30.0 milestone Nov 5, 2025
@github-actions github-actions bot added the bug label Nov 5, 2025
@afdesk
Copy link
Contributor Author

afdesk commented Nov 5, 2025

@simar7 I remember you have a meticulous attitude toward testing, and I was thinking about how to add tests here.

A simple table-driven test for createEnvAndVolumeForGcr doesn’t make much sense in this context.

Once we resolve issues #2595 and #2769, we’ll be able to add a more meaningful test that covers multiple containers.

@afdesk afdesk marked this pull request as ready for review November 5, 2025 06:12
@afdesk afdesk requested a review from simar7 as a code owner November 5, 2025 06:12
Comment on lines 777 to +784
func createEnvandVolumeForGcr(env *[]corev1.EnvVar, volumeMounts *[]corev1.VolumeMount, volumes *[]corev1.Volume, registryPasswordKey, secretName *string) {
const googlecredVolumeName = "gcrvol"
for _, v := range *volumes {
if v.Name == googlecredVolumeName {
return
}
}

Copy link
Member

Choose a reason for hiding this comment

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

To test something like this where the number of invocations that happen is what we're ultimately interested in, we could add a test where we keep track of invocations. Knowing how many times we expect createEnvandVolumeForGcr to get called can let us know if the logic is working as expected or if there's a bug.

In this example, with an early return in case of an existing mount, we should have less invocations than before introducing this change.

@simar7
Copy link
Member

simar7 commented Dec 5, 2025

@simar7 I remember you have a meticulous attitude toward testing, and I was thinking about how to add tests here.

A simple table-driven test for createEnvAndVolumeForGcr doesn’t make much sense in this context.

Once we resolve issues #2595 and #2769, we’ll be able to add a more meaningful test that covers multiple containers.

I left an idea https://github.com/aquasecurity/trivy-operator/pull/2801/files#r2591586352

@afdesk afdesk requested a review from simar7 December 10, 2025 13:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

A vulnerabilityreport scan job cannot be started due to volume name duplicates in GKE

2 participants