Skip to content

Conversation

@jaer-tsun
Copy link
Contributor

@jaer-tsun jaer-tsun commented Sep 28, 2021

Reason for Change:
Adding DNS & IMDS NAT Policy for Windows AKS-Swift Scenario

  • DNS traffic to NC Primary IP
  • IMDS traffic to Host/VM IP

Issue Fixed:

Requirements:

Notes:

@jaer-tsun jaer-tsun force-pushed the nat-dns-aksswift branch 8 times, most recently from 697c235 to e530125 Compare September 30, 2021 22:13
@jaer-tsun jaer-tsun changed the title fix: Add DNS NAT Policy for Windows AKS-Swift Scenario fix: Add NAT Policies for Windows AKS-Swift Scenario Sep 30, 2021
@pjohnst5
Copy link
Contributor

pjohnst5 commented Oct 1, 2021

@jaer-tsun Thanks for the PR, do you know what endpoints we use of the 169.254.169.254 (IMDS) ip?

I just know that anything other than 169.254.169.254/metadata is blocked now

Edit: I guess it might just be 169.254.169.254/metadata that we use looking at utils.go, we're good

metadataURL = "http://169.254.169.254/metadata/instance?api-version=2017-08-01&format=json"

@pjohnst5
Copy link
Contributor

pjohnst5 commented Oct 1, 2021

@tamilmani1989 can you take a peek at this PR?

Basically we're saying now, that if CNI windows is Aks-Swift (aka, not multitenancy, and CNS as Ipam), then add two snat rules for IMDS and DNS Ips to be routed through the host

@tamilmani1989
Copy link
Member

tamilmani1989 commented Oct 1, 2021

@neaggarwMS @jaer-tsun can we set executionMode to AKS-Swift to determine if cni running in aks-swift or not.. instead of checking checking multitenancy and ipam type as azure-cns?

Copy link
Contributor

@pjohnst5 pjohnst5 left a comment

Choose a reason for hiding this comment

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

Just a comment

@jaer-tsun jaer-tsun requested a review from pjohnst5 October 14, 2021 16:56
Copy link
Collaborator

@rbtr rbtr left a comment

Choose a reason for hiding this comment

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

minor syntactic and style things

@jaer-tsun jaer-tsun force-pushed the nat-dns-aksswift branch 4 times, most recently from d2b47ae to 80bb513 Compare October 15, 2021 23:17
rbtr
rbtr previously requested changes Oct 15, 2021
@rbtr rbtr dismissed their stale review October 18, 2021 17:39

all my comments have been addressed

@jaer-tsun jaer-tsun force-pushed the nat-dns-aksswift branch 2 times, most recently from 1de0c42 to 8c3e481 Compare October 19, 2021 21:09
@jaer-tsun jaer-tsun force-pushed the nat-dns-aksswift branch 3 times, most recently from b71b461 to ea08c12 Compare October 20, 2021 18:09
Copy link
Member

@tamilmani1989 tamilmani1989 left a comment

Choose a reason for hiding this comment

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

lgtm 🚢

@jaer-tsun jaer-tsun merged commit 0108572 into Azure:master Oct 20, 2021
@jaer-tsun jaer-tsun deleted the nat-dns-aksswift branch October 20, 2021 20:58
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.

5 participants