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

fix: check manifests kinds when running a Job #4348

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

boonware
Copy link

@boonware boonware commented Nov 23, 2022

Issue

spinnaker/spinnaker#6769

Description

  • Allow a manifest file to contain more than one embedded manifest when running a Job. This supports use cases such as a Job manifest bundled with a ConfigMap manifest in the same file. Strictly speaking, it is a manifest file containing more than one Job definition that is the problem. This is solved by checking the kind of each manifest in the file, and only throwing an exception if more than one Job kind is found.
  • If necessary, further filtering could be added to check for other executable kinds, see https://kubernetes.io/docs/reference/kubernetes-api/workload-resources/

@boonware boonware changed the title fix: check manifests types fix: check manifests kinds when running a Job Nov 23, 2022
@boonware
Copy link
Author

@dbyron-sf Could I get a workflow approval, please?

@dbyron-sf
Copy link
Contributor

No need to tag me on PRs. I see them fly by.

I'm not sure I agree that this is a fix though. Seems more like a feature/change in behavior, and one that could break folks who depend on this validation. I think I'd prefer that additional work go into filtering so it's possible to leave run job stages alone and deploy the additional manifests (presumably without the job) in a deploy manifest stage.

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