Cluster-api-provider-azure: Implement Azure bastion host

[justaugustus]

/kind feature

Describe the solution you'd like
Adding a bastion node will allow secure access to nodes, without having to rely on NAT rules on the public load balancer, laying the groundwork for non-public capz scenarios.

Use the capa bastion as a reference point: https://github.com/kubernetes-sigs/cluster-api-provider-aws/blob/master/pkg/cloud/aws/services/ec2/bastion.go

https://docs.microsoft.com/en-us/azure/bastion/bastion-overview

Related: #104

/priority important-soon
/milestone v1alpha

[k8s-ci-robot]

@justaugustus: The provided milestone is not valid for this repository. Milestones in this repository: [baseline, mvp, next, v1alpha1]

Use /milestone clear to clear the milestone.

In response to this:

/kind feature

Describe the solution you'd like
Adding a bastion node will allow secure access to nodes, without having to rely on NAT rules on the public load balancer, laying the groundwork for non-public capz scenarios.

Related: #104

/priority important soon
/milestone v1alpha

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.

[justaugustus]

/priority important-soon
/milestone v1alpha1

[justaugustus]

/help

[tahsinrahman]

I can work on this. How should it look like?

What i can think of now is,

• A bastion host with public ip
• A new subnet for bastion host, with security group rule that allows all traffice at port 22
• Remove ssh rule from controlplane sg
• Remove ssh nat rule from load balancer

[justaugustus]

@tahsinrahman -- Appreciate the help! What you've described looks like a great first pass. Additionally, let's make sure that the bastion uses the same OS as the Cluster API machines (Ubuntu 18.04).

Tag me on the PR when you're ready for someone to review. :)

[justaugustus]

/remove-help

[fejta-bot]

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]

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

[fejta-bot]

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]

@fejta-bot: Closing this issue.

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.

[justaugustus]

/reopen
/remove-lifecycle rotten

[k8s-ci-robot]

@justaugustus: Reopened this issue.

In response to this:

/reopen
/remove-lifecycle rotten

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.

[justaugustus]

/assign @juan-lee

[fejta-bot]

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]

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

[CecileRobertMichon]

/remove-lifecycle rotten
/help

[CecileRobertMichon]

/assign @cpanato

[cpanato]

/remove-help

[CecileRobertMichon]

Pasting @voor 's feature request from #528 here:

Azure Bastion is a neat feature that allows secure connections to the individual VMs without exposing their SSH ports to the internet. The default configuration should add in the necessary address space, AzureBastionSubnet, and vnet-bastion setup to allow for this to work without any additional configuration. Documentation should be updated to reflect this is a more secure and preferable way to connect to hosts instead of exposing them to the internet.

[k8s-ci-robot]

@justaugustus: The label(s) priority/important, priority/soon cannot be applied, because the repository doesn't have them

In response to this:

/kind feature

Describe the solution you'd like
Adding a bastion node will allow secure access to nodes, without having to rely on NAT rules on the public load balancer, laying the groundwork for non-public capz scenarios.

Use the capa bastion as a reference point: https://github.com/kubernetes-sigs/cluster-api-provider-aws/blob/master/pkg/cloud/aws/services/ec2/bastion.go

Related: #104

/priority important soon
/milestone v1alpha

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]

@justaugustus: The label(s) priority/important, priority/soon cannot be applied, because the repository doesn't have them

In response to this:

/kind feature

Describe the solution you'd like
Adding a bastion node will allow secure access to nodes, without having to rely on NAT rules on the public load balancer, laying the groundwork for non-public capz scenarios.

Use the capa bastion as a reference point: https://github.com/kubernetes-sigs/cluster-api-provider-aws/blob/master/pkg/cloud/aws/services/ec2/bastion.go

https://docs.microsoft.com/en-us/azure/bastion/bastion-overview

Related: #104

/priority important soon
/milestone v1alpha

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.

[voor]

Only thing I want to add is to make sure we're not confusing bastion-the-concept with Azure Bastion-the-capability, since Azure Bastion is explicitly allowed in certain environments, whereas punching a hole for port 22 through a VM is not, the original issue was directed towards the hole-punching approach.

[CecileRobertMichon]

@voor I think focusing on implementing Azure Bastion makes sense. I don't think it was available when this issue was originally opened (I just edited the description and title to mention Azure bastion).

since Azure Bastion is explicitly allowed in certain environments

You mean https://docs.microsoft.com/en-us/azure/bastion/bastion-overview#regions right?

[cpanato]

/milestone v0.4.9

[k8s-ci-robot]

@cpanato: You must be a member of the kubernetes-sigs/cluster-api-provider-azure-maintainers GitHub team to set the milestone. If you believe you should be able to issue the /milestone command, please contact your Cluster API Provider Azure Maintainers and have them propose you as an additional delegate for this responsibility.

In response to this:

/milestone v0.4.9

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.

[CecileRobertMichon]

/milestone next

[fejta-bot]

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

[voor]

/remove-lifecycle stale

[fejta-bot]

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-contributor-experience at kubernetes/community.
/lifecycle stale

[whites11]

Are we still interested in this feature? I could work on it.

[devigned]

/remove-lifecycle stale

[devigned]

@whites11, I we are still interested in this feature. We had thought about using Azure Bastion to implement this, but I think it might be best to just use a regular VM. If you are interested, we'd appreciate the collaboration.

/assign @whites11

[whites11]

Thanks for the answer @devigned
+1 for the VM approach.
I will think how to proceed and make a proposal.

[devigned]

Awesome! Looking forward to the proposal.

There is some existing work in the space:

• Existing Azure Bastion Service code: https://github.com/kubernetes-sigs/cluster-api-provider-azure/tree/master/azure/services/bastionhosts
• https://github.com/kubernetes-sigs/cluster-api-provider-azure/pull/175
• https://github.com/kubernetes-sigs/cluster-api-provider-azure/pull/708

If you want to riff on some stuff, please don't hesitate to bring it up in slack or a github discussion.

[whites11]

Before I begin, let me share my current idea to get early feedback.
I can imagine three different ways of implementing a bastion solution for a CAPZ cluster:

• Azure Bastion feature
• Dedicated Machine
• Use master node

There are pros and cons for all of the above and I can imagine use cases for all of them.
Idea: why don't we make this configurable?
I was thinking about something like this:

apiVersion: infrastructure.cluster.x-k8s.io/v1alpha3
kind: AzureCluster
metadata:
  name: d3m3v
  namespace: default
spec:
  bastionSpec:
    azureBastion:
      name: "azurebastion1"
      subnet: *SubnetSpec
      publicIP:
        name: "azurebastion1-publicIP"
    dedicatedHost:
      machineRef: *ObjectReference
      publicIP:
        name: "azurebastion1-publicIP"
    masterNode:
      securityGroup:
        name: "existing-sg-name"
        ingressRules: []ingressRule
...

A bastionSpec field in the AzureClusterSpec that contains one object for each approach defined above.
This allows us to either use the same approach as PersistentVolumes do (you can define one and only one of the 3 fields at the same time) or maybe allow for multiple bastion strategies to be in place at the same time.

This is just a sketch, but if you think it can be a viable solution I can further think about it, provide draft CRDs and move on from here.

[CecileRobertMichon]

I like the idea of allowing different implementations and the way you've structured the spec.

Only feedback I have is on the naming of things, dedicatedHost can be confused with https://azure.microsoft.com/en-us/services/virtual-machines/dedicated-host/ and we should avoid using the word master in masterNode per k8s WG Naming .

[whites11]

Thanks for the comment @CecileRobertMichon
What are the next steps for me after addressing your naming concerns? Do I need a CAEP? Or should I begin with the PR directly? (sorry noob CAPZer here :sweat_smile:)

[CecileRobertMichon]

Since this is an opt-in feature and not a big redesign, and given that you've already presented the intended changes above, I think it's okay to proceed without a CAEP. We should agree on naming here before proceeding to a PR though. Do you have any suggestions?

Also, for implementation, what do you think about breaking out the changes into 3 PRs, one for each bastion approach so we can review and merge them one at a time?

[whites11]

Since this is an opt-in feature and not a big redesign, and given that you've already presented the intended changes above, I think it's okay to proceed without a CAEP.

Thanks for clarifying

We should agree on naming here before proceeding to a PR though. Do you have any suggestions?

Let me think about it and I will post another comment.

Also, for implementation, what do you think about breaking out the changes into 3 PRs, one for each bastion approach so we can review and merge them one at a time?

Exactly what I was thinking to do

[whites11]

OK what about this:

apiVersion: infrastructure.cluster.x-k8s.io/v1alpha3
kind: AzureCluster
metadata:
  name: d3m3v
  namespace: default
spec:
  bastionSpec:
    azureBastion:
      name: "azurebastion1"
      subnet: *SubnetSpec
      publicIP:
        name: "azurebastion1-publicIP"
    azureMachine:
      machineRef: *ObjectReference
      publicIP:
        name: "azurebastion1-publicIP"
    controlPlaneNode:
      securityGroup:
        name: "existing-sg-name"
        ingressRules: []ingressRule
...

azureBastion and azureMachine refer to two azure services (Azure Bastion and Azure Virtual Machine obviously) so I feel ok prefixing the two keys with azure.
Control Plane Node is what wg-naming suggests to indicate one of what I previously called "master node". So it makes sense to me to call the object key controlPlaneNode.

WDYAT?

[CecileRobertMichon]

LGTM

@nader-ziada @shysank @mboersma any opinions?

[cpanato]

lgtm as well
and thanks for moving forward with this issue <3

[nader-ziada]

the naming in the last comment looks good to me, so does proceeding directly to PR(s)

thanks @whites11

[mboersma]

👍🏼 to all of this. The bastionSpec @whites11 sketched out looks reasonable, and I don't think a CAEP is required here.

[whites11]

Thanks all for the feedback.
I'm close to getting the first PR (the one using existing code for AzureBastion) to a usable state.
News soon.

[devigned]

/close

Fixed via #1300

[k8s-ci-robot]

@devigned: Closing this issue.

In response to this:

/close

Fixed via #1300

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.

[whites11]

/close

Fixed via #1300

Wait. This issue discussed 3 strategies for implementing bastion access to workload clusters. I implemented only one in #1300. I'm working on the second one, should we keep tracking it in this issue?

[devigned]

/reopen

@whites11, you are right. I was a little too quick to close.

[k8s-ci-robot]

@devigned: Reopened this issue.

In response to this:

/reopen

@whites11, you are right. I was a little too quick to 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.

[whites11]

I am working on the second strategy, the one that uses virtual machines as bastion hosts to access nodes.
The approved CRD structure is as follows:

apiVersion: infrastructure.cluster.x-k8s.io/v1alpha3
kind: AzureCluster
metadata:
  name: d3m3v
  namespace: default
spec:
  bastionSpec:
    azureMachine:
      machineRef: *ObjectReference
      publicIP:
        name: "azurebastion1-publicIP"
...

but now that I am actually working on it I believe my proposal was wrong and incomplete.
I will try to explain the conclusions I came up with while trying to implement this, an updated CR example and some details here and there in order to collect feedback and ideas.

GOAL: goal is to have one (or more) virtual machine(s) acting as a bastion to ssh/rdp into kubernetes nodes rather than using one of the masters or the AzureBastion feature.

To adhere to the KISS principle my idea is to reuse existing reconciliation controllers for that.
In practice that means creating 3 CRs:

1. A Secret holding the cloud-init config file to bootstrap the bastion VM with the needed config (has to be provided by user and can't be defaulted any way)
2. An AzureMachineTemplate for each bastion that describes how the VM should be (size, OS, disks, etc). Can be defaulted.
3. A MachineDeployment to bind the previous 2 CRs together and make CAPI/CAPZ controllers reconcile and create the VMs. Can be defaulted.

From a UX point of view, we only need to provide a way to define the Secret (point 1 above) and optionally the AzureMachineTemplate.
The CR could be something like:

apiVersion: infrastructure.cluster.x-k8s.io/v1alpha3
kind: AzureCluster
metadata:
  name: d3m3v
  namespace: default
spec:
  bastionSpec:
    azureMachine:
      bootstrapDataSecretName: "<secret name>"
      azureMachineTemplates: []
      replicas: 2
...

With the above information, the azure cluster controller can create 2 azure machine templates and 2 machine deployments and then the creation of VMs should happen automagically.

Why a slice of azuremachinetemplates you might ask? Because having multiple bastion hosts only makes sense if those VMs are placed in different availability zones. Since the failureDomain field is a scalar in the AzureMachineTemplate CR, I need to have different templates for different availability zones.

WDYT in general about this idea?

[devigned]

+1 on this thus far. However, I think there are a couple things missing that I would like to see.
1) What are the security rules associated to ingress for the bastion public IP? How will someone set these ingress rules? Is that public IP going to be in its own network security group or the cluster's NSG? I would like to better understand the Azure network infrastructure resources and how access will be constrained.
2) I think it would be super useful to be able to provide SSH public keys for the deployment, so users can easily provide ssh access to the bastion.

Might be a good idea to create a proposal for this work.

[whites11]

Thanks a lot for your input @devigned

+1 on this thus far. However, I think there are a couple things missing that I would like to see.

1. What are the security rules associated to ingress for the bastion public IP? How will someone set these ingress rules? Is that public IP going to be in its own network security group or the cluster's NSG? I would like to better understand the Azure network infrastructure resources and how access will be constrained.

Absolutely agree. Network configurability is key and I will think about it in the CAEP.

1. I think it would be super useful to be able to provide SSH public keys for the deployment, so users can easily provide ssh access to the bastion.

I am not sure I agree with this. On one end it would make things easier for what we could consider the most common use case but on the other hand we would make it harder for more complex ones. But let's discuss this in the CAEP I guess.

Might be a good idea to create a proposal for this work.

Will begin working on this ASAP.
BTW I noticed my idea so far is provider independent and that's awesome IMHO.