Skip to content
This repository has been archived by the owner on Oct 20, 2023. It is now read-only.

New Resource: IdentityBinding #248

Merged

Conversation

keithmattix
Copy link
Contributor

@keithmattix keithmattix commented Mar 13, 2022

This PR adds a new TrafficAccess resource: IdentityBinding. Note that a new release with this change would advance TrafficAccess to v1alpha4 and modify TrafficTarget to accept a(n) (optional) group along with a kind for source and destination resouces.

Signed-off-by: Keith Mattix II <[email protected]>
@keithmattix keithmattix force-pushed the feature/identitybinding branch from 0f2fd35 to c70e90d Compare March 14, 2022 01:43
apis/traffic-access/v1alpha4/traffic-access.md Outdated Show resolved Hide resolved
apis/traffic-access/v1alpha4/traffic-access.md Outdated Show resolved Hide resolved
Comment on lines +349 to +352
- Other types of policy - having policy around retries, timeouts and rate limits
would be great. This specific object only manages access control. As policy
for these examples would be HTTP specific, there needs to be a HTTP specific
policy object created.

Choose a reason for hiding this comment

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

A resource like https://gateway-api.sigs.k8s.io/v1alpha2/references/policy-attachment/ might be a reasonable way of handling this in the future?

Comment on lines +344 to +347
- Ingress policy - assuming clients present the correct identity, this *should*
work for some kind of ingress. Unfortunately, it does not cover many of the
common use cases (filtering by hostname) and will need to be expanded to cover
this use case.

Choose a reason for hiding this comment

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

This might be a relevant case to consider proposing TrafficTarget at some point in the future (maybe with a different name to better reflect its authorization purpose?) as an extension to the Kubernetes Gateway API, which I believe intends to support some forms of authentication, but AFAIK doesn't yet have any way of representing those (JWT identity is probably something that would be desirable to add at that point).

apis/traffic-access/v1alpha4/traffic-access.md Outdated Show resolved Hide resolved
Also add some clarification around security model

Signed-off-by: Keith Mattix II <[email protected]>
@keithmattix keithmattix force-pushed the feature/identitybinding branch from 33d7527 to 2340800 Compare March 25, 2022 17:42
@keithmattix keithmattix requested review from stefanprodan and removed request for stefanprodan March 25, 2022 17:42
Copy link
Collaborator

@nicholasjackson nicholasjackson left a comment

Choose a reason for hiding this comment

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

Looks great Keith

stefanprodan
stefanprodan previously approved these changes Mar 28, 2022
Copy link
Contributor

@stefanprodan stefanprodan left a comment

Choose a reason for hiding this comment

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

LGTM

Thanks @keithmattix 🏅

Multiple podLabelSelectors doesn't seem necessary and the semantics
around such configuration are confusing. If the need arises for
multiple selectors, we can reconsider, but let's keep it simple
for now

Signed-off-by: Keith Mattix II <[email protected]>
@keithmattix keithmattix force-pushed the feature/identitybinding branch from 6ef1d7e to 4314b5f Compare March 30, 2022 15:59
Copy link
Member

@bridgetkromhout bridgetkromhout left a comment

Choose a reason for hiding this comment

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

Thanks!

@keithmattix keithmattix merged commit 87904c8 into servicemeshinterface:main May 4, 2022
@keithmattix keithmattix deleted the feature/identitybinding branch May 4, 2022 19:02
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants