gatekeeper-library icon indicating copy to clipboard operation
gatekeeper-library copied to clipboard

The example of disallowed/allowed ingress resources in the unique ingress host example has incorrect hostnames

Open WilliamRockwellEvans opened this issue 1 year ago • 9 comments

Link: https://open-policy-agent.github.io/gatekeeper-library/website/validation/uniqueingresshost/ The constraint template example uses a simple host_1 == host_2 logic, but the allowed and disallowed examples don't share the same host name -- accordingly the disallowed resource isn't blocked.

example-allowed: example-allowed-host.example.com example-allowed-host1.example.com

example-disallowed: example-host.example.com

example-disallowed2: example-host2.example.com example-host3.example.com

WilliamRockwellEvans avatar Feb 12 '24 19:02 WilliamRockwellEvans

Hi Everyone!

There are three independent tests cases/samples, the resources aren't shared between them:

https://github.com/open-policy-agent/gatekeeper-library/blob/master/library/general/uniqueingresshost/suite.yaml

  - name: example-allowed
    object: samples/unique-ingress-host/example_allowed.yaml
    assertions:
    - violations: no
  - name: example-disallowed
    object: samples/unique-ingress-host/example_disallowed.yaml
    inventory:
    - samples/unique-ingress-host/example_inventory_disallowed.yaml
    assertions:
    - violations: yes
  - name: example-disallowed2
    object: samples/unique-ingress-host/example_disallowed2.yaml
    inventory:
    - samples/unique-ingress-host/example_inventory_disallowed2.yaml
    assertions:
    - violations: yes

example-allowed: (0 violations)

example-disallowed: (1 violation)

example-disallowed2: (1 violation)

@JaydipGabani - These tests appears to be working as intended to me, can you confirm?

apeabody avatar Feb 16 '24 17:02 apeabody

@apeabody I think the issue refers to the information that is directly user-facing on the library website where the available allowed and disallowed examples appear to have not been violating the policy.

As in if user if trying out the policy,

  • user applies the policy
  • user applies allowed example
  • then user applies disallowed example expecting it to generate violation/get denied - the host does not match currently between allowed and disallowed - but the disallowed object gets created leaving the user thinking the policy is faulty

The tests are working as intended as inventory is in direct conflict with examples however inventory objects are not part of the policy documentation on the website. So on the website, naming examples allowed and disallowed makes it so that user might think these provided examples are supposed to be conflicting - that is the case for many policies that do not require sync as far as I can tell.

JaydipGabani avatar Feb 16 '24 18:02 JaydipGabani

Thanks @JaydipGabani!

I think the issue refers to the information that is directly user-facing on the library website where the available allowed and disallowed examples appear to have not been violating the policy.

Got it, makes total sense. Would it perhaps be more sustainable to automate inclusion of the "missing inventory resources into the documentation examples? This gap could potentially apply to all templates/samples which use data.inventory, and it might not be feasible to manually "fix" all of them in this manner. Nor do we have any sort of automated testing to avoid drift in the future.

apeabody avatar Feb 16 '24 18:02 apeabody

@apeabody Agreed! I took a look and there are some policies that uses data,inventory. For instance - hpa policy requires additional nginx deployment to exists out of box.

Do you suggest merging the associated PR for this and fixing this for now and opening a new issue to fix a greater problem? or do you want to close this issue without merging the PR and open a new issue to fix a greater problem?

JaydipGabani avatar Feb 16 '24 20:02 JaydipGabani

Do you suggest merging the associated PR for this and fixing this for now and opening a new issue to fix a greater problem? or do you want to close this issue without merging the PR and open a new issue to fix a greater problem?

No concerns with the current PR. :) More a point that the tests are working as intended, so the PR is a workaround for what is really a documentation gap. So we should have an open issue to fix that regardless.

apeabody avatar Feb 16 '24 20:02 apeabody

This issue/PR has been automatically marked as stale because it has not had recent activity. It will be closed in 14 days if no further activity occurs. Thank you for your contributions.

stale[bot] avatar Apr 17 '24 00:04 stale[bot]

still valid

JaydipGabani avatar Apr 17 '24 18:04 JaydipGabani

This issue/PR has been automatically marked as stale because it has not had recent activity. It will be closed in 14 days if no further activity occurs. Thank you for your contributions.

stale[bot] avatar Jun 16 '24 20:06 stale[bot]

Not stale

JaydipGabani avatar Jun 17 '24 19:06 JaydipGabani

This issue/PR has been automatically marked as stale because it has not had recent activity. It will be closed in 14 days if no further activity occurs. Thank you for your contributions.

stale[bot] avatar Aug 16 '24 22:08 stale[bot]