Implement bastion host #175

tahsinrahman · 2019-04-07T16:05:42Z

Which issue(s) this PR fixes
Fixes #165

Implement Bastion Host

This change is

k8s-ci-robot · 2019-04-07T16:05:55Z

Hi @tahsinrahman. Thanks for your PR.

I'm waiting for a kubernetes-sigs or kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

tahsinrahman · 2019-04-07T16:06:09Z

@justaugustus

justaugustus · 2019-04-07T16:38:35Z

/ok-to-test

awesomenix · 2019-04-10T13:00:55Z

pkg/cloud/azure/actuators/cluster/reconciler.go

+		return errors.Wrapf(err, "failed to createorupdate bastion public ip for cluster %s", s.scope.Cluster.Name)
+	}
+
+	bastionNicSpec := &networkinterfaces.Spec{


can this be part of machine actuator itself, so we can define 3 different roles, node, controlplane and bastion. so we can specify this as part of cluster spec itself instead of hard coding values here

awesomenix · 2019-04-10T13:02:41Z

pkg/cloud/azure/services/networkinterfaces/networkinterfaces.go

@@ -37,7 +39,7 @@ type Spec struct {
 	StaticIPAddress          string
 	PublicLoadBalancerName   string
 	InternalLoadBalancerName string
-	NatRule                  int


So bastion VM is just a flavour of created cluster, some users may not want additional vm. So please add support for both scenarios

awesomenix · 2019-04-10T13:03:57Z

pkg/cloud/azure/services/networkinterfaces/networkinterfaces.go

@@ -120,6 +117,20 @@ func (s *Service) CreateOrUpdate(ctx context.Context, spec azure.Spec) error {
 	}
 	nicConfig.LoadBalancerBackendAddressPools = &backendAddressPools

+	if nicSpec.PublicIPName != "" {


please reuse the current public load balancer and reuse the nat rule for bastion vm instead of control plane vm

tahsinrahman · 2019-04-10T16:05:38Z

@awesomenix so, clusterSpec will have a field named bastionSpec, and if this field is nil, it won't affect current system, but if not nil, we will create bastion vm defined in bastionSpec?

awesomenix · 2019-04-10T16:20:01Z

@awesomenix so, clusterSpec will have a field named bastionSpec, and if this field is nil, it won't affect current system, but if not nil, we will create bastion vm defined in bastionSpec?

Sorry i meant this should be part of machinelist example here https://github.com/kubernetes-sigs/cluster-api-provider-azure/blob/master/cmd/clusterctl/examples/azure/machines.yaml.template

You can see we define controlplane and node roles, there should be a bastion role as well which you have defined in few places in this PR.

Cluster spec defines that this is a locked down cluster, in azure we call it private cluster, but not sure the terminology in aws. But you can define a flag which mentions that this is IsPrivate
The idea is that let machine actuator machine the lifecycle of the bastion VM (self recovery/maintenance), rather than cluster actuator managing the lifecycle of bastion vm and hardcoding certain values (like image name etc).
This way you can reuse most of the components already existing in cluster (publicip publiclb etc).
You can use nat rule 0 which is ssh access for bastion rather than control plane vm, remove the health probes if IsPrivate is set to true
Also you might need to change the api server ip and also kubeconfig so that clusterctl succeeds provisioning and is able to access the kubernetes cluster. <- probably this might be done through bastion vm, but not sure.

Could you please do a quick write up on how this will look for end user?

Create workflow
Future runtime access (how this will be done)

tahsinrahman · 2019-04-10T17:15:54Z

so, as far as i understand,

user can deploy a machine with role bastion
if user deploys bastion machine, reconcile the load balancer rule to redirect ssh traffics to bastion vm
if user deletes bastion machine, reconcile the load balancer rule to allow ssh traffic on controlplane vms

For private clusters,

the load balancer will only be used to ssh into the bastion host, as we're removing health probes?

justaugustus · 2019-04-10T20:15:43Z

@tahsinrahman -- please hold off on changing anything until I have the opportunity to review this. I should be able to tonight or by tomorrow afternoon at the latest.

justaugustus · 2019-04-17T08:09:46Z

@tahsinrahman -- overall, this is a great start! I had some refactoring to do elsewhere in the repo, which has caused your branch to have some conflicts.

Can you please rebase this and then I can give a thorough review?

alexeldeib · 2019-06-06T11:10:21Z

@tahsinrahman @justaugustus what is the status of this PR? Is it moving ahead? HA is kind of depending on fixing the networking rules, which this PR touches.

k8s-ci-robot · 2019-06-20T17:53:16Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: tahsinrahman
To complete the pull request process, please assign justaugustus
You can assign the PR to them by writing /assign @justaugustus in a comment when ready.

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

Needs approval from an approver in each of these files:

OWNERS

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

tahsinrahman · 2019-06-20T17:56:12Z

@alexeldeib @justaugustus i've updated the pr. it now uses load balancer inbound nat rules to connect to bastion host.

@awesomenix should bastion node be defined as machine? doesn't a machine represent a kubernetes node?

fejta-bot · 2019-09-18T19:33:10Z

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle stale

fejta-bot · 2019-11-15T06:59:43Z

Stale issues rot after 30d of inactivity.
Mark the issue as fresh with /remove-lifecycle rotten.
Rotten issues close after an additional 30d of inactivity.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle rotten

k8s-ci-robot · 2019-12-03T22:00:44Z

@tahsinrahman: The following tests failed, say /retest to rerun them all:

Test name	Commit	Details	Rerun command
pull-cluster-api-provider-azure-build	`ba21188`	link	`/test pull-cluster-api-provider-azure-build`
pull-cluster-api-provider-azure-test	`ba21188`	link	`/test pull-cluster-api-provider-azure-test`
pull-cluster-api-provider-azure-verify	`ba21188`	link	`/test pull-cluster-api-provider-azure-verify`
pull-cluster-api-provider-azure-e2e	`ba21188`	link	`/test pull-cluster-api-provider-azure-e2e`

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

fejta-bot · 2020-01-02T22:01:04Z

Rotten issues close after 30d of inactivity.
Reopen the issue with /reopen.
Mark the issue as fresh with /remove-lifecycle rotten.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/close

k8s-ci-robot · 2020-01-02T22:01:11Z

@tahsinrahman: PR needs rebase.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

k8s-ci-robot · 2020-01-02T22:01:12Z

@fejta-bot: Closed this PR.

In response to this:

Rotten issues close after 30d of inactivity.
Reopen the issue with /reopen.
Mark the issue as fresh with /remove-lifecycle rotten.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/close

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

k8s-ci-robot added release-note Denotes a PR that will be considered when it comes time to generate release notes. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. area/provider/azure Issues or PRs related to azure provider labels Apr 7, 2019

k8s-ci-robot requested review from awesomenix and juan-lee April 7, 2019 16:05

k8s-ci-robot added needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Apr 7, 2019

k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Apr 7, 2019

tahsinrahman mentioned this pull request Apr 8, 2019

REQUEST: New membership for tahsinrahman kubernetes/org#697

Closed

6 tasks

k8s-ci-robot added sig/apps Categorizes an issue or PR as relevant to SIG Apps. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Apr 8, 2019

awesomenix suggested changes Apr 10, 2019

View reviewed changes

alexeldeib mentioned this pull request Apr 11, 2019

Control plane NSG is open to the internet #104

Closed

k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Apr 13, 2019

tahsinrahman force-pushed the add-bastion branch from 3faa68a to d1eb077 Compare April 18, 2019 17:02

k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Apr 18, 2019

k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Apr 24, 2019

Implement bastion host

c2b7417

tahsinrahman force-pushed the add-bastion branch from d1eb077 to c2b7417 Compare June 20, 2019 17:53

k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 20, 2019

Fix test

6bacc04

tahsinrahman force-pushed the add-bastion branch from 8e07aef to 6bacc04 Compare June 20, 2019 18:16

Fix delete, bastion should be deleted before subnet

ba21188

k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Sep 18, 2019

justaugustus added this to the next milestone Oct 3, 2019

k8s-ci-robot added lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Nov 15, 2019

k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jan 2, 2020

k8s-ci-robot closed this Jan 2, 2020

devigned mentioned this pull request Apr 1, 2021

Implement Azure bastion host #165

Closed

CecileRobertMichon removed this from the next milestone May 4, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement bastion host #175

Implement bastion host #175

tahsinrahman commented Apr 7, 2019 •

edited

Loading

k8s-ci-robot commented Apr 7, 2019

tahsinrahman commented Apr 7, 2019

justaugustus commented Apr 7, 2019

awesomenix Apr 10, 2019

awesomenix Apr 10, 2019

awesomenix Apr 10, 2019

tahsinrahman commented Apr 10, 2019

awesomenix commented Apr 10, 2019

tahsinrahman commented Apr 10, 2019

justaugustus commented Apr 10, 2019

justaugustus commented Apr 17, 2019

alexeldeib commented Jun 6, 2019

k8s-ci-robot commented Jun 20, 2019

tahsinrahman commented Jun 20, 2019 •

edited

Loading

fejta-bot commented Sep 18, 2019

fejta-bot commented Nov 15, 2019

k8s-ci-robot commented Dec 3, 2019

fejta-bot commented Jan 2, 2020

k8s-ci-robot commented Jan 2, 2020

k8s-ci-robot commented Jan 2, 2020

Implement bastion host #175

Implement bastion host #175

Conversation

tahsinrahman commented Apr 7, 2019 • edited Loading

k8s-ci-robot commented Apr 7, 2019

tahsinrahman commented Apr 7, 2019

justaugustus commented Apr 7, 2019

awesomenix Apr 10, 2019

Choose a reason for hiding this comment

awesomenix Apr 10, 2019

Choose a reason for hiding this comment

awesomenix Apr 10, 2019

Choose a reason for hiding this comment

tahsinrahman commented Apr 10, 2019

awesomenix commented Apr 10, 2019

tahsinrahman commented Apr 10, 2019

justaugustus commented Apr 10, 2019

justaugustus commented Apr 17, 2019

alexeldeib commented Jun 6, 2019

k8s-ci-robot commented Jun 20, 2019

tahsinrahman commented Jun 20, 2019 • edited Loading

fejta-bot commented Sep 18, 2019

fejta-bot commented Nov 15, 2019

k8s-ci-robot commented Dec 3, 2019

fejta-bot commented Jan 2, 2020

k8s-ci-robot commented Jan 2, 2020

k8s-ci-robot commented Jan 2, 2020

tahsinrahman commented Apr 7, 2019 •

edited

Loading

tahsinrahman commented Jun 20, 2019 •

edited

Loading