fix: add missing Argo CronWorkflow to transforms

[kevin-hanselman]

Description
Other Argo Workflows CRDs with images are already allowed in the source. This PR adds the missing CronWorkflow CRD.

User facing changes
Skaffold will now appropriately transform the images mentioned in CronWorkflow resources.

[google-cla]

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

For more information, open the CLA check for this pull request.

[MarlonGamez]

hi there @kevin-hanselman! Thanks for opening this. Before moving forward, would you be willing to try using our new config that allows you to specify kubernetes resources to be modifiable by skaffold? Our docs for this can be found here.

[kevin-hanselman]

@MarlonGamez I am using that now, but I think it's a very awkward and surprising solution given that the rest of the Argo resources are built-in to Skaffold already. What is the concern with adding CronWorkflow to Skaffold?

[MarlonGamez]

@kevin-hanselman I don't think we have any concern about it, just that we'd like to find a comfortable solution for people in the future to avoid them having to go through the friction of creating a PR for skaffold to work with their resources. You're right that we have the other argo resources here so I think it does make sense to add this one for this PR. I'll approve and merge this one.

Can I ask what you think would improve the awkwardness/surprise of the solution we have now?

[kevin-hanselman]

Can I ask what you think would improve the awkwardness/surprise of the solution we have now?

@MarlonGamez Merging this PR 😄 Really though: The surprise comes from expecting Skaffold to treat CronWorkflow resources the same as other Argo resources. The awkwardness comes from the workaround -- using a custom resource selector for CronWorkflow resources specifically. Another dev looking at my Skaffold config might ask: "Why aren't you adding other Argo resources to the resourceSelector section?" (That is, if I hadn't linked to this PR in the file. 😄)

I think the resource selector is a huge win, by the way. Please don't mistake this PR for criticism of that concept. It's the inconsistency of handling Argo resources that's the issue for me. If for some reason this PR wouldn't get merged, the next best solution, IMO, would be to remove all Argo resources from the built-in transforms and lean on the resource selector wholesale.

[MarlonGamez]

Ahh okay, makes sense, thanks for clarifying! Appreciate the response :)

[tejal29]

dummy FB

[codecov]

Codecov Report

Merging #7205 (58b7709) into main (290280e) will decrease coverage by 1.96%.
The diff coverage is 56.96%.

@@            Coverage Diff             @@
##             main    #7205      +/-   ##
==========================================
- Coverage   70.48%   68.51%   -1.97%     
==========================================
  Files         515      560      +45     
  Lines       23150    26495    +3345     
==========================================
+ Hits        16317    18153    +1836     
- Misses       5776     7090    +1314     
- Partials     1057     1252     +195     

Impacted Files	Coverage Δ
cmd/skaffold/app/cmd/deploy.go	52.00% <ø> (-1.85%)	⬇️
cmd/skaffold/app/cmd/dev.go	84.61% <0.00%> (ø)
cmd/skaffold/app/cmd/render.go	36.66% <0.00%> (-4.72%)	⬇️
cmd/skaffold/skaffold.go	0.00% <0.00%> (ø)
cmd/skaffold/app/cmd/inspect_tests.go	62.50% <14.28%> (-1.14%)	⬇️
cmd/skaffold/app/cmd/lsp.go	28.12% <28.12%> (ø)
cmd/skaffold/app/cmd/fix.go	68.85% <40.00%> (-7.62%)	⬇️
cmd/skaffold/app/cmd/lint.go	42.85% <42.85%> (ø)
cmd/skaffold/app/cmd/find_configs.go	48.88% <50.00%> (+0.24%)	⬆️
cmd/skaffold/app/skaffold.go	76.19% <70.00%> (-8.43%)	⬇️
... and 224 more

📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more

[MarlonGamez]

lgtm