Management of non Docker or S2I build types #17
Conversation
pkg/webhook/webhook.go
Outdated
| @@ -124,7 +124,7 @@ func getAdmissionResponseForBuild(ar *v1beta1.AdmissionReview, quayIntegration * | |||
|
|
|||
| quayRegistryHostname, err := quayIntegration.GetRegistryHostname() | |||
|
|
|||
| if build.Spec.CommonSpec.Output.To.Kind != "ImageStreamTag" { | |||
| if buildv1.JenkinsPipelineBuildStrategyType == build.Spec.Strategy.Type || build.Spec.CommonSpec.Output.To == nil || build.Spec.CommonSpec.Output.To.Kind != "ImageStreamTag" { | |||
There was a problem hiding this comment.
If I read the docs correct it seems like only the strategies (build.spec.strategy.type) of type sourceStrategy or dockerStrategy or customStrategy qualify for build outputs to be images. So maybe a reverse check on them is more future-proof than intercepting Jenkins builds. To be safe I'd also check if the spec.output element exists.
| @@ -124,7 +124,7 @@ func getAdmissionResponseForBuild(ar *v1beta1.AdmissionReview, quayIntegration * | |||
|
|
|||
| quayRegistryHostname, err := quayIntegration.GetRegistryHostname() | |||
|
|
|||
| if buildv1.JenkinsPipelineBuildStrategyType == build.Spec.Strategy.Type || build.Spec.CommonSpec.Output.To == nil || build.Spec.CommonSpec.Output.To.Kind != "ImageStreamTag" { | |||
| if (build.Spec.Strategy.DockerStrategy == nil && build.Spec.Strategy.SourceStrategy == nil) || build.Spec.CommonSpec.Output.To.Kind != "ImageStreamTag" { | |||
There was a problem hiding this comment.
What about custom build strategies? Other than that, assuming that Docker and S2I strategies mandate that spec.ouput.to is not NIL I think this lgtm
There was a problem hiding this comment.
Custom Build strategies is a big question on whether it should be supported which is why I went down my original approach to really only exclude JenkinsPipeline and then do checks on whether certain fields are present.
Output is not a pointer so there will not be an issue of hitting a NPE
There was a problem hiding this comment.
Ack on custom builds not being supported. I think we should not break the system when we encounter such a build. People could be mixing several types of build configs in a namespace. Not supporting it by gracefully ignoring those seems the best way to me.
There was a problem hiding this comment.
As the logic stands now, custom strategies would not have their output image destinations mutated and would be ignored. Is that the expectation that we would want to deliver?
Addresses how to handle other build types that are not compatible with producing output images