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

Add unique ingress host to library #253

Merged
merged 6 commits into from
Oct 12, 2019

Conversation

ritazh
Copy link
Member

@ritazh ritazh commented Sep 25, 2019

Signed-off-by: Rita Zhang rita.z.zhang@gmail.com

@tsandall
Copy link
Member

tsandall commented Sep 26, 2019 via email

@maxsmythe
Copy link
Contributor

Sadly, I'm not sure they are always present.

@ritazh ritazh force-pushed the library-ingress-conflict branch from fc8da42 to 02aa1d1 Compare September 30, 2019 18:10
@ritazh ritazh changed the title [WIP] Add unique ingress host to library Add unique ingress host to library Sep 30, 2019
@ritazh ritazh requested a review from maxsmythe September 30, 2019 21:29
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 after group is fixed and confirmation we validated that Ingress has input.review.name populated

library/general/uniqueingresshost/template.yaml Outdated Show resolved Hide resolved
library/general/uniqueingresshost/src_test.rego Outdated Show resolved Hide resolved
@ritazh ritazh force-pushed the library-ingress-conflict branch from ac36249 to f8a5c9a Compare October 3, 2019 02:11
@ritazh ritazh requested review from maxsmythe and tsandall October 3, 2019 02:12
@ritazh
Copy link
Member Author

ritazh commented Oct 7, 2019

bump

Copy link
Member

@tsandall tsandall left a comment

Choose a reason for hiding this comment

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

Took another pass. A few minor comments. LGTM.

library/general/uniqueingresshost/constraint.yaml Outdated Show resolved Hide resolved
library/general/uniqueingresshost/src.rego Outdated Show resolved Hide resolved
@ritazh ritazh requested a review from maxsmythe October 8, 2019 21:43
other := data.inventory.namespace[ns][otherapi]["Ingress"][name]
otherapi == apis[_]
other := data.inventory.namespace[ns][otherapiversion]["Ingress"][name]
re_match("^(extensions|networking.k8s.io).+$", otherapiversion)
Copy link
Contributor

Choose a reason for hiding this comment

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

add a / before the .+$ to guarantee we aren't catching groups like: extensions.something.org

@maxsmythe
Copy link
Contributor

LGTM

@maxsmythe maxsmythe self-requested a review October 12, 2019 02:04
Signed-off-by: Rita Zhang <rita.z.zhang@gmail.com>
Signed-off-by: Rita Zhang <rita.z.zhang@gmail.com>
Signed-off-by: Rita Zhang <rita.z.zhang@gmail.com>
Signed-off-by: Rita Zhang <rita.z.zhang@gmail.com>
Signed-off-by: Rita Zhang <rita.z.zhang@gmail.com>
Signed-off-by: Rita Zhang <rita.z.zhang@gmail.com>
@ritazh ritazh force-pushed the library-ingress-conflict branch from 8c1dceb to 99ff692 Compare October 12, 2019 03:56
@ritazh ritazh merged commit 70f65c4 into open-policy-agent:master Oct 12, 2019
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.

3 participants