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

Preserve unknown fields in ByPodStatus #117

Conversation

julianKatz
Copy link
Contributor

A Gatekeeper e2e test is currently failing in
open-policy-agent/gatekeeper#1286 due to the use of the operations
field. This field is added by Gatekeeper, under a ConstraintTemplate's
byPodsStatus. This field relied on the v1beta1 CRD behavior, where
unknown fields are preserved by default.

As this field fundamentally has to do with gatekeeper, it's better not
to define it in the Constraint Framework. Other CF consumers have no
need for the Operations field.

Even better, we may eventually define ConstraintTemplateSpec in
Constraint Framework consumers, rather than in the library itself.

For now, adding x-kubernetes-preserve-unknown-fields: true to
ByPodStatus will solve our problem and maintain the existing behavior.

Contributes to open-policy-agent/gatekeeper#550

Signed-off-by: juliankatz juliankatz@google.com

Copy link
Contributor

@maxsmythe maxsmythe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@julianKatz julianKatz force-pushed the byPodStatus-operations-unknown-fields branch from e7c79b6 to 3be88ce Compare May 18, 2021 00:50
@maxsmythe
Copy link
Contributor

@ritazh @shomron @sozercan this PR is blocking for handling the v1beta1 CRD deprecation, it fixes an e2e failure in this PR:

open-policy-agent/gatekeeper#1286

A Gatekeeper e2e test is currently failing in
open-policy-agent/gatekeeper#1286 due to the use of the `operations`
field.  This field is added by Gatekeeper, under a ConstraintTemplate's
`byPodsStatus`.  This field relied on the v1beta1 CRD behavior, where
unknown fields are preserved by default.

As this field fundamentally has to do with gatekeeper, it's better not
to define it in the Constraint Framework.  Other CF consumers have no
need for the Operations field.

Even better, we may eventually define ConstraintTemplateSpec in
Constraint Framework consumers, rather than in the library itself.

For now, adding `x-kubernetes-preserve-unknown-fields: true` to
ByPodStatus will solve our problem and maintain the existing behavior.

Contributes to open-policy-agent/gatekeeper#550

Signed-off-by: juliankatz <juliankatz@google.com>
@julianKatz julianKatz force-pushed the byPodStatus-operations-unknown-fields branch from 3be88ce to 02fe192 Compare May 18, 2021 18:33
@maxsmythe maxsmythe merged commit becaa0e into open-policy-agent:master May 18, 2021
@shomron
Copy link

shomron commented May 18, 2021

👍🏻

julianKatz added a commit to julianKatz/gatekeeper that referenced this pull request May 18, 2021
This PR changes the flags passed to controller-gen to generate v1 CRDs
for the CRDs defined in this repository.

It also includes changes from open-policy-agent/frameworks#114, which
update the ConstraintTemplate CRD to v1, and
open-policy-agent/frameworks#117, which allowed Gatekeeper to add the
Operations field to byPodStatus, which isn't built in to the byPodStatus
golang struct.

Contributes to open-policy-agent#550

Signed-off-by: juliankatz <juliankatz@google.com>
julianKatz added a commit to julianKatz/frameworks that referenced this pull request May 18, 2021
The previous PR (open-policy-agent#117) put the `x-kubernetes-preserve-unknown-fields:
true` key/value pair one level too deep.  This moves it alongside
`byPodStatus`' `properties`.

Contributes to open-policy-agent/gatekeeper#550

Signed-off-by: juliankatz <juliankatz@google.com>
julianKatz added a commit to julianKatz/frameworks that referenced this pull request May 18, 2021
The previous PR (open-policy-agent#117) put the `x-kubernetes-preserve-unknown-fields:
true` key/value pair one level too deep.  This moves it alongside
`byPodStatus`' `properties`.

Contributes to open-policy-agent/gatekeeper#550

Signed-off-by: juliankatz <juliankatz@google.com>
julianKatz added a commit to julianKatz/frameworks that referenced this pull request May 18, 2021
The previous PR (open-policy-agent#117) put the `x-kubernetes-preserve-unknown-fields:
true` key/value pair one level too deep.  This moves it alongside
`byPodStatus`' `properties`.

Contributes to open-policy-agent/gatekeeper#550

Signed-off-by: juliankatz <juliankatz@google.com>
julianKatz added a commit to julianKatz/gatekeeper that referenced this pull request May 18, 2021
This PR changes the flags passed to controller-gen to generate v1 CRDs
for the CRDs defined in this repository.

It also includes changes from open-policy-agent/frameworks#114, which
update the ConstraintTemplate CRD to v1, and
open-policy-agent/frameworks#117, which allowed Gatekeeper to add the
Operations field to byPodStatus, which isn't built in to the byPodStatus
golang struct.

Contributes to open-policy-agent#550

Signed-off-by: juliankatz <juliankatz@google.com>
julianKatz added a commit to julianKatz/gatekeeper that referenced this pull request May 18, 2021
This PR changes the flags passed to controller-gen to generate v1 CRDs
for the CRDs defined in this repository.

It also includes changes from open-policy-agent/frameworks#114, which
update the ConstraintTemplate CRD to v1, and
open-policy-agent/frameworks#117, which allowed Gatekeeper to add the
Operations field to byPodStatus, which isn't built in to the byPodStatus
golang struct.

Contributes to open-policy-agent#550

Signed-off-by: juliankatz <juliankatz@google.com>
julianKatz added a commit to julianKatz/gatekeeper that referenced this pull request May 20, 2021
This PR changes the flags passed to controller-gen to generate v1 CRDs
for the CRDs defined in this repository.

It also includes changes from open-policy-agent/frameworks#114, which
update the ConstraintTemplate CRD to v1, and
open-policy-agent/frameworks#117, which allowed Gatekeeper to add the
Operations field to byPodStatus, which isn't built in to the byPodStatus
golang struct.

Contributes to open-policy-agent#550

Signed-off-by: juliankatz <juliankatz@google.com>
julianKatz added a commit to open-policy-agent/gatekeeper that referenced this pull request May 20, 2021
This PR changes the flags passed to controller-gen to generate v1 CRDs
for the CRDs defined in this repository.

It also includes changes from open-policy-agent/frameworks#114, which
update the ConstraintTemplate CRD to v1, and
open-policy-agent/frameworks#117, which allowed Gatekeeper to add the
Operations field to byPodStatus, which isn't built in to the byPodStatus
golang struct.

Contributes to #550

Signed-off-by: juliankatz <juliankatz@google.com>
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.

4 participants