Skip to content
This repository has been archived by the owner on Jun 13, 2024. It is now read-only.

Add NSG exceptional rules for the K8s master SSH service #46

Merged
merged 4 commits into from
Nov 20, 2017

Conversation

allxiao
Copy link
Contributor

@allxiao allxiao commented Nov 17, 2017

This PR addresses the issue #45. To test, set environment variable export MS_CORP=1 before provisioning the resources.

There's a web job scanning for the NSG in MS subscriptions, and apply restrictions on the access to the MS corp network. As a result, if we deploy the resources to the MS subscriptions, they cannot access each other so the CI/CD cannot proceed. (There's only a few exceptional cases like port 443).

We cannot add NSG rules to allow access from a fixed IP address set. We can get the service IP for those pre-allocated services, like the Jenkins master, but we cannot get the public IP for the Jenkins slaves provisioned by the Kubernetes plugin, because they may not have public IP at all.

As a result, an exceptional rule with Internet source to the port 22 was added. The web job deletes the wildcard rule if the port is fixed to 22, so we have our rule defined as 21-23.

It's working as I tested, however, it is not an ideal solution.

@allxiao
Copy link
Contributor Author

allxiao commented Nov 20, 2017

@ZhijunZhao Could you help to review this? I don't have permission to update the reviewers.

@ZhijunZhao
Copy link
Contributor

@ArieShout Working on it.

@@ -77,6 +83,13 @@ create_secrets_in_kubernetes ${w_eu_group} ${ACS_NAME}

print_banner 'Deploy Jenkins cluster if not exist...'
deploy_jenkins ${jenkins_group} ${ACS_NAME}
if [[ -n "$JENKINS_IP_ADDRESS" ]]; then
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you remove this part?

@@ -64,6 +64,12 @@ wait_till_kubernetes_created ${w_eu_group} ${ACS_NAME}
wait_till_kubernetes_created ${jenkins_group} ${ACS_NAME}
[[ $? -ne 0 ]] && return 1

if [[ -n "$MS_CORP" ]]; then
Copy link
Contributor

Choose a reason for hiding this comment

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

Please update the notes of running source provision.sh to reflect this change, saying internal users need to set this variable.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated.

@ZhijunZhao ZhijunZhao merged commit f698400 into microsoft:master Nov 20, 2017
@allxiao allxiao deleted the nsg branch November 20, 2017 03:47
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.

2 participants