adding a new container policy for konflux pipeline execution #1247
Conversation
acornett21
added
the
do-not-merge/work-in-progress
openshift-ci
bot
removed
the
do-not-merge/work-in-progress
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: acornett21 The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci
bot
added
the
approved
acornett21
force-pushed
the
konflux_cmd
branch
from
5d80495 to
c5c2993
Compare
|
from change #1247: |
acornett21
added
the
do-not-merge/work-in-progress
acornett21
changed the title
openshift-ci
bot
removed
the
do-not-merge/work-in-progress
|
from change #1247: |
| @@ -38,6 +38,7 @@ func printChecks(w io.Writer) { | |||
| "automatically applied for container checks if preflight determines a scratch exception flag has been added to your Red Hat Connect project")) | |||
| fmt.Fprintln(w, formattedPolicyBlock("Container Scratch (Root) Exception", engine.ScratchRootContainerPolicy(context.TODO()), | |||
| "automatically applied for container checks if preflight determines scratch and root exception flags have both been added to your Red Hat Connect project")) | |||
| // todo-adam should we add konflux here? | |||
There was a problem hiding this comment.
My gut says no, but I'm open to the idea.
There was a problem hiding this comment.
My gut said no as well, we'll let others chime in.
|
from change #1247: |
|
from change #1247: |
acornett21
force-pushed
the
konflux_cmd
branch
2 times, most recently
from
0af9585 to
c381cfa
Compare
|
from change #1247: |
| @@ -0,0 +1,9 @@ | |||
| package container | |||
There was a problem hiding this comment.
This file seems extraneous. If you want this to be a library function that can be re-used by both "prohibited" and "required" label checks, might be better to move this to another package. Here, it feels like cruft.
There was a problem hiding this comment.
@bcrochet suggested this, so that it's easier to find, since I had originally kept it in the original labels check.
There was a problem hiding this comment.
Feels a bit unnecessary to me, but if you want to keep it as a library function, that's fine. Just put it in some internal/... directory vs. here.
I'll defer to @bcrochet here, though, to decide if leaving this here is fine.
|
|
||
| var _ check.Check = &HasProhibitedLabelsCheck{} | ||
|
|
||
| type HasProhibitedLabelsCheck struct{} |
There was a problem hiding this comment.
This naming is unclear to me. This check asserts that labels don't have prohibited value, which today translates into trademark violations. This check does not check that given label keys are missing, as the name might suggest.
There was a problem hiding this comment.
This has always been a problem with our naming, I can change it to HasNoProhibitedLabelsCheck
There was a problem hiding this comment.
Eh. I didn't catch that this was the old name. I don't love it, given what it's doing now, but I suppose I can't say much given the old name was basically this.
There was a problem hiding this comment.
Up to you how you want to handle this.
|
from change #1247: |
Signed-off-by: Adam D. Cornett <[email protected]>
|
from change #1247: |
Motivation
Today, for konflux all policies are ran and since the introduction of the Red Hat trademark violation policies, we have been unable to ship a new image to be used in konflux. This means they are missing out on some bugfixes and performance enhancements.
Changes
Introduce a newkonfluxsub command iepreflight check konflux. This cmd builds out the properOptionsto be used in/by the engine.PolicyKonfluxand corresponding container policy with matching tests.HasRequiredLabelsCheckto only check for required labels.HasProhibitedLabelsCheckto check for Red Hat Trademark violations within a containers labels, and exclude this from thePolicyKonflux.ResolveSubmitterto also look forkonfluxflag so aNoopSubmitteris returned even if a user tried to provide a pyixs component/key and thekonfluxenv.