fix: Add values to PR generator

[dudo]

Fixes #11408

This seems as though it'd be something that we'll want to roll out to all the generators. It seems more flexible than adding something like branchParamPrefix, and less disruptive than adding a new key for pr_branch which is superfluous, and may end up being deprecated at some point in the future.

If there's appetite to add it to the rest of the generators, I'm happy to add that to the PR, but it's beyond the scope of the linked issue.

Checklist:

• Either (a) I've created an enhancement proposal and discussed it with the community, (b) this is a bug fix, or (c) this does not need to be in the release notes.
• The title of the PR states what changed and the related issues number (used for the release note).
• The title of the PR conforms to the Toolchain Guide
• I've included "Closes [ISSUE #]" or "Fixes [ISSUE #]" in the description to automatically close the associated issue.
• I've updated both the CLI and UI to expose my feature, or I plan to submit a second PR with them.
• Does this PR require documentation updates?
• I've updated documentation as required by this PR.
• I have signed off all my commits as required by DCO
• I have written unit and/or e2e tests for my change. PRs without these are unlikely to be merged.
• My build is green (troubleshooting builds).
• My new feature complies with the feature status guidelines.
• I have added a brief description of why this PR is necessary and/or what this PR solves.
• Optional. My organization is added to USERS.md.
• Optional. For bug fixes, I've indicated what older releases this fix should be cherry-picked into (this may or may not happen depending on risk/complexity).

[bunnyshell]

🔴 Preview Environment stopped on Bunnyshell

See: Environment Details | Pipeline Logs

Available commands (reply to this comment):

• 🔵 /bns:start to start the environment
• 🚀 /bns:deploy to redeploy the environment
• ❌ /bns:delete to remove the environment

[dudo]

(That DCO check is silly... my commit is verified...)

[dudo]

We can close #16507 once this is merged.

[reggie-k]

Thanks for the PR!
How are the changes in test/container/Dockerfile file related to the generator values?

[dudo]

Hey @reggie-k !

Those changes are the only way I could actually run make codegen on my machine. It was failing with a message about permissions in the /go folder, and this ensures there's a folder to have permissions on, then adds them.

[reggie-k]

Hey @reggie-k !

Those changes are the only way I could actually run make codegen on my machine. It was failing with a message about permissions in the /go folder, and this ensures there's a folder to have permissions on, then adds them.

Could you run make codegen-local instead? If that works, I would suggest to open another issue for the permissions related error for make codegen and undo the changes for the test Dockerfile.

[dudo]

I don't love installing a bunch of random dependencies on my machine, especially with this using the older go path. Containerization keeps things... contained 😅.

Are you (or anyone) able to run make codegen successfully? I can't imagine I'm missing some other local environment var that's not getting picked up. The changes I made addressed some warnings (case of AS and ENV syntax) then just ensured /go/src is present and owned by the user provided. Pretty innocuous (even though it was blocking me). If you insist, I'll open another PR - just trying to keep the ball rolling here.

[reggie-k]

Totally legit to stick to the virtualized chain, I take your point.

From the perspective of maintainers' tracking of changes, and users visibility, the changes in the Dockerfile should go to a separate PR.
I opened an issue to track this separately:
#21623
Your PR for the Dockerfile will now be even smaller, having this being merged already:
#21556
And can be merged quickly to unblock this PR (even though all the codegen generated manifests are already in this PR and are not really blocked)

[dudo]

Sorry for the delay, here. Updated!

[codecov]

Codecov Report

Attention: Patch coverage is 25.00000% with 3 lines in your changes missing coverage. Please review.

Please upload report for BASE (master@544aea1). Learn more about missing BASE report.

Files with missing lines	Patch %	Lines
applicationset/generators/pull_request.go	25.00%	2 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff            @@
##             master   #21557   +/-   ##
=========================================
  Coverage          ?   53.52%           
=========================================
  Files             ?      339           
  Lines             ?    57185           
  Branches          ?        0           
=========================================
  Hits              ?    30606           
  Misses            ?    23945           
  Partials          ?     2634           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.