Skip to content
This repository was archived by the owner on Jan 11, 2023. It is now read-only.

Remove security rules from NSG.#3695

Closed
marrobi wants to merge 1 commit intoAzure:masterfrom
marrobi:remove-nsg-rules
Closed

Remove security rules from NSG.#3695
marrobi wants to merge 1 commit intoAzure:masterfrom
marrobi:remove-nsg-rules

Conversation

@marrobi
Copy link
Contributor

@marrobi marrobi commented Aug 16, 2018

What this PR does / why we need it:
Please look at #3693 first, as this is a priority, although this PR could replace #3693 .

As per #3693 the allow_kube_tls rule may not be required. In addition there is no need to allow SSH or RDP through the NSG if there are no masters - they are annotated "Allow SSH traffic to master"/"Allow RDP traffic to master".

The case with masters is dealt with in k8s/kubernetesmasterresources.t as per:

 {{if not IsHostedMaster}}
      ,{{template "k8s/kubernetesmasterresources.t" .}}
    {{else}}

If a user needs to RDP or SSH onto nodes, then they should be aware they are opening the ports, and removing a layer of security.

In addition the SSH rule is flagged as a risk by Azure Security Centre.

Which issue this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close that issue when PR gets merged): fixes #

Special notes for your reviewer:
Again I do not believe any adverse affect, although users may be relying on these rules to administer agent pool nodes.

If applicable:

  • documentation
  • unit tests
  • tested backward compatibility (ie. deploy with previous version, upgrade with this branch)

@marrobi marrobi self-assigned this Aug 16, 2018
@ghost ghost added the in progress label Aug 16, 2018
@acs-bot
Copy link

acs-bot commented Aug 16, 2018

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: marrobi
To fully approve this pull request, please assign additional approvers.
We suggest the following additional approver: mboersma

If they are not already assigned, you can assign the PR to them by writing /assign @mboersma in a comment when ready.

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@acs-bot acs-bot added the size/M label Aug 16, 2018
@jackfrancis
Copy link
Member

@marrobi Would changing the priority of these built-in rules allow additional rules that collide with 443/22 to work?

@marrobi
Copy link
Contributor Author

marrobi commented Aug 16, 2018 via email

@CecileRobertMichon
Copy link
Contributor

Closing since this was covered by #4164

@ghost ghost removed the in progress label Jan 8, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants