Skip to content

Conversation

@pjohnst5
Copy link
Contributor

Reason for Change:
In cni manager, allow setting cns url and enable exact match for pod name

@pjohnst5 pjohnst5 changed the title Make multitenant configs plumb-able in cni manager Multitenant configs for cni manager Mar 28, 2022
@rbtr
Copy link
Collaborator

rbtr commented Mar 28, 2022

what's the use case?

@pjohnst5 pjohnst5 marked this pull request as ready for review March 31, 2022 19:00
@pjohnst5
Copy link
Contributor Author

@rbtr
Copy link
Collaborator

rbtr commented Mar 31, 2022

i worry about allowing arbitrary URL injection at runtime, that could provide a path to privilege escalation

@pjohnst5
Copy link
Contributor Author

Is there something else I could do?
in multitenancy cni, a cns url is possible to specify, and I have to set it as something other than the default in this case.
idk

@pjohnst5
Copy link
Contributor Author

pjohnst5 commented Apr 1, 2022

What do you say @rbtr ? Given the scenario, is it okay to merge?

Copy link
Contributor

@jaer-tsun jaer-tsun left a comment

Choose a reason for hiding this comment

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

lgtm

@pjohnst5 pjohnst5 enabled auto-merge (squash) April 6, 2022 20:10
@pjohnst5
Copy link
Contributor Author

pjohnst5 commented Apr 6, 2022

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@rbtr
Copy link
Collaborator

rbtr commented Apr 6, 2022

What do you say @rbtr ? Given the scenario, is it okay to merge?

I'm not blocking 🙂

@rbtr rbtr disabled auto-merge April 14, 2022 22:18
@rbtr rbtr merged commit 7fb08d9 into Azure:master Apr 14, 2022
@pjohnst5 pjohnst5 deleted the cni-manager-augmentation branch April 14, 2022 22:19
matmerr pushed a commit to matmerr/azure-container-networking that referenced this pull request Jun 29, 2022
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