Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[WIP] Initial implementation of ACM certificate for API server ELB #5414

Merged
merged 7 commits into from Jul 19, 2018
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
8 changes: 8 additions & 0 deletions cmd/kops/create_cluster.go
Expand Up @@ -123,6 +123,9 @@ type CreateClusterOptions struct {
// Specify API loadbalancer as public or internal
APILoadBalancerType string

// Specify the SSL certificate to use for the API loadbalancer. Currently only supported in AWS.
APISSLCertificate string

// Allow custom public master name
MasterPublicName string

Expand Down Expand Up @@ -327,6 +330,7 @@ func NewCmdCreateCluster(f *util.Factory, out io.Writer) *cobra.Command {
cmd.Flags().StringVar(&options.NodeTenancy, "node-tenancy", options.NodeTenancy, "The tenancy of the node group on AWS. Can be either default or dedicated.")

cmd.Flags().StringVar(&options.APILoadBalancerType, "api-loadbalancer-type", options.APILoadBalancerType, "Sets the API loadbalancer type to either 'public' or 'internal'")
cmd.Flags().StringVar(&options.APISSLCertificate, "api-ssl-certificate", options.APISSLCertificate, "Currently only supported in AWS. Sets the ARN of the SSL Certificate to use for the API server loadbalancer.")

// Allow custom public master name
cmd.Flags().StringVar(&options.MasterPublicName, "master-public-name", options.MasterPublicName, "Sets the public master public name")
Expand Down Expand Up @@ -1052,6 +1056,10 @@ func RunCreateCluster(f *util.Factory, out io.Writer, c *CreateClusterOptions) e
}
}

if c.APISSLCertificate != "" {
cluster.Spec.API.LoadBalancer.SSLCertificate = c.APISSLCertificate
}

// Use Strict IAM policy and allow AWS ECR by default when creating a new cluster
cluster.Spec.IAM = &api.IAMSpec{
AllowContainerRegistry: true,
Expand Down
1 change: 1 addition & 0 deletions docs/cli/kops_create_cluster.md
Expand Up @@ -66,6 +66,7 @@ kops create cluster [flags]
```
--admin-access strings Restrict API access to this CIDR. If not set, access will not be restricted by IP. (default [0.0.0.0/0])
--api-loadbalancer-type string Sets the API loadbalancer type to either 'public' or 'internal'
--api-ssl-certificate string Currently only supported in AWS. Sets the ARN of the SSL Certificate to use for the API server loadbalancer.
--associate-public-ip Specify --associate-public-ip=[true|false] to enable/disable association of public IP for master ASG and nodes. Default is 'true'.
--authorization string Authorization mode to use: AlwaysAllow or RBAC (default "RBAC")
--bastion Pass the --bastion flag to enable a bastion instance group. Only applies to private topology.
Expand Down
1 change: 1 addition & 0 deletions pkg/apis/kops/cluster.go
Expand Up @@ -282,6 +282,7 @@ type LoadBalancerAccessSpec struct {
Type LoadBalancerType `json:"type,omitempty"`
IdleTimeoutSeconds *int64 `json:"idleTimeoutSeconds,omitempty"`
AdditionalSecurityGroups []string `json:"additionalSecurityGroups,omitempty"`
SSLCertificate string `json:"sslCertificate,omitempty"`
}

// KubeDNSConfig defines the kube dns configuration
Expand Down
1 change: 1 addition & 0 deletions pkg/apis/kops/v1alpha1/cluster.go
Expand Up @@ -281,6 +281,7 @@ type LoadBalancerAccessSpec struct {
Type LoadBalancerType `json:"type,omitempty"`
IdleTimeoutSeconds *int64 `json:"idleTimeoutSeconds,omitempty"`
AdditionalSecurityGroups []string `json:"additionalSecurityGroups,omitempty"`
SSLCertificate string `json:"sslCertificate,omitempty"`
}

// KubeDNSConfig defines the kube dns configuration
Expand Down
2 changes: 2 additions & 0 deletions pkg/apis/kops/v1alpha1/zz_generated.conversion.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions pkg/apis/kops/v1alpha2/cluster.go
Expand Up @@ -282,6 +282,7 @@ type LoadBalancerAccessSpec struct {
Type LoadBalancerType `json:"type,omitempty"`
IdleTimeoutSeconds *int64 `json:"idleTimeoutSeconds,omitempty"`
AdditionalSecurityGroups []string `json:"additionalSecurityGroups,omitempty"`
SSLCertificate string `json:"sslCertificate,omitempty"`
}

type KubeDNSConfig struct {
Expand Down
2 changes: 2 additions & 0 deletions pkg/apis/kops/v1alpha2/zz_generated.conversion.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion pkg/client/simple/vfsclientset/commonvfs.go
Expand Up @@ -31,7 +31,7 @@ import (
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/runtime/schema"
"k8s.io/kops/pkg/acls"
kops "k8s.io/kops/pkg/apis/kops"
"k8s.io/kops/pkg/apis/kops"
"k8s.io/kops/pkg/apis/kops/v1alpha2"
"k8s.io/kops/pkg/kopscodecs"
"k8s.io/kops/util/pkg/vfs"
Expand Down
3 changes: 2 additions & 1 deletion pkg/kubeconfig/create_kubecfg.go
Expand Up @@ -85,7 +85,8 @@ func BuildKubecfg(cluster *kops.Cluster, keyStore fi.Keystore, secretStore fi.Se

b.Context = clusterName

{
// add the CA Cert to the kubeconfig only if we didn't specify a SSL cert for the LB
if cluster.Spec.API.LoadBalancer.SSLCertificate == "" {
Copy link
Member

Choose a reason for hiding this comment

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

Ah - this is giving us a panic in e2e... (e.g. https://k8s-gubernator.appspot.com/build/kubernetes-jenkins/pr-logs/pull/kops/5414/pull-kops-e2e-kubernetes-aws/6969/ )

I think you need:

if cluster.Spec.API == nil || cluster.Spec.API.LoadBalancer == nil || cluster.SPec.API.LoadBalancer.SSLCertificate == "" {

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I've seen the test a couple of days ago and yes I think that should be enough to fix, haven't had the time to do it do even if it's super quick, I was pretty busy these days... but the good news is: I will probably get it done tomorrow 😄

cert, _, _, err := keyStore.FindKeypair(fi.CertificateId_CA)
if err != nil {
return nil, fmt.Errorf("error fetching CA keypair: %v", err)
Expand Down
14 changes: 10 additions & 4 deletions pkg/model/awsmodel/api_loadbalancer.go
Expand Up @@ -102,6 +102,14 @@ func (b *APILoadBalancerBuilder) Build(c *fi.ModelBuilderContext) error {
idleTimeout = time.Second * time.Duration(*lbSpec.IdleTimeoutSeconds)
}

listeners := map[string]*awstasks.LoadBalancerListener{
"443": {InstancePort: 443},
}

if lbSpec.SSLCertificate != "" {
listeners["443"] = &awstasks.LoadBalancerListener{InstancePort: 443, SSLCertificateID: lbSpec.SSLCertificate}
}

elb = &awstasks.LoadBalancer{
Name: s("api." + b.ClusterName()),
Lifecycle: b.Lifecycle,
Expand All @@ -110,10 +118,8 @@ func (b *APILoadBalancerBuilder) Build(c *fi.ModelBuilderContext) error {
SecurityGroups: []*awstasks.SecurityGroup{
b.LinkToELBSecurityGroup("api"),
},
Subnets: elbSubnets,
Listeners: map[string]*awstasks.LoadBalancerListener{
"443": {InstancePort: 443},
},
Subnets: elbSubnets,
Listeners: listeners,

// Configure fast-recovery health-checks
HealthCheck: &awstasks.LoadBalancerHealthCheck{
Expand Down
1 change: 0 additions & 1 deletion pkg/validation/validate_cluster.go
Expand Up @@ -71,7 +71,6 @@ func hasPlaceHolderIP(clusterName string) (bool, error) {
if err != nil {
return true, fmt.Errorf("unable to parse Kubernetes cluster API URL: %v", err)
}

hostAddrs, err := net.LookupHost(apiAddr.Hostname())
if err != nil {
return true, fmt.Errorf("unable to resolve Kubernetes cluster API URL dns: %v", err)
Expand Down
50 changes: 25 additions & 25 deletions upup/pkg/fi/cloudup/awstasks/load_balancer.go
Expand Up @@ -58,12 +58,12 @@ type LoadBalancer struct {

Scheme *string

HealthCheck *LoadBalancerHealthCheck
AccessLog *LoadBalancerAccessLog
//AdditionalAttributes []*LoadBalancerAdditionalAttribute
HealthCheck *LoadBalancerHealthCheck
AccessLog *LoadBalancerAccessLog
ConnectionDraining *LoadBalancerConnectionDraining
ConnectionSettings *LoadBalancerConnectionSettings
CrossZoneLoadBalancing *LoadBalancerCrossZoneLoadBalancing
SSLCertificateID string
}

var _ fi.CompareWithID = &LoadBalancer{}
Expand All @@ -73,17 +73,17 @@ func (e *LoadBalancer) CompareWithID() *string {
}

type LoadBalancerListener struct {
InstancePort int
InstancePort int
SSLCertificateID string
}

func (e *LoadBalancerListener) mapToAWS(loadBalancerPort int64) *elb.Listener {
return &elb.Listener{
LoadBalancerPort: aws.Int64(loadBalancerPort),

Protocol: aws.String("TCP"),

InstanceProtocol: aws.String("TCP"),
Protocol: aws.String("SSL"),
InstanceProtocol: aws.String("SSL"),
InstancePort: aws.Int64(int64(e.InstancePort)),
SSLCertificateId: aws.String(e.SSLCertificateID),
}
}

Expand Down Expand Up @@ -334,16 +334,6 @@ func (e *LoadBalancer) Find(c *fi.Context) (*LoadBalancer, error) {
actual.AccessLog.S3BucketPrefix = lbAttributes.AccessLog.S3BucketPrefix
}

// We don't map AdditionalAttributes - yet
//var additionalAttributes []*LoadBalancerAdditionalAttribute
//for index, additionalAttribute := range lbAttributes.AdditionalAttributes {
// additionalAttributes[index] = &LoadBalancerAdditionalAttribute{
// Key: additionalAttribute.Key,
// Value: additionalAttribute.Value,
// }
//}
//actual.AdditionalAttributes = additionalAttributes

actual.ConnectionDraining = &LoadBalancerConnectionDraining{}
if lbAttributes.ConnectionDraining.Enabled != nil {
actual.ConnectionDraining.Enabled = lbAttributes.ConnectionDraining.Enabled
Expand Down Expand Up @@ -381,7 +371,7 @@ func (e *LoadBalancer) Find(c *fi.Context) (*LoadBalancer, error) {
// 1. We don't want to force a rename of the ELB, because that is a destructive operation
// 2. We were creating ELBs with insufficiently qualified names previously
if fi.StringValue(e.LoadBalancerName) != fi.StringValue(actual.LoadBalancerName) {
glog.V(2).Infof("Resuing existing load balancer with name: %q", actual.LoadBalancerName)
glog.V(2).Infof("Reusing existing load balancer with name: %q", actual.LoadBalancerName)
e.LoadBalancerName = actual.LoadBalancerName
}

Expand Down Expand Up @@ -453,11 +443,7 @@ func (s *LoadBalancer) CheckChanges(a, e, changes *LoadBalancer) error {
return fi.RequiredField("ConnectionDraining.Enabled")
}
}
//if e.ConnectionSettings != nil {
// if e.ConnectionSettings.IdleTimeout == nil {
// return fi.RequiredField("ConnectionSettings.IdleTimeout")
// }
//}

if e.CrossZoneLoadBalancing != nil {
if e.CrossZoneLoadBalancing.Enabled == nil {
return fi.RequiredField("CrossZoneLoadBalancing.Enabled")
Expand Down Expand Up @@ -558,6 +544,20 @@ func (_ *LoadBalancer) RenderAWS(t *awsup.AWSAPITarget, a, e, changes *LoadBalan
}

if changes.Listeners != nil {

elbDescription, err := findLoadBalancerByLoadBalancerName(t.Cloud, loadBalancerName)
if err != nil {
return fmt.Errorf("error getting load balancer by name: %v", err)
}

if elbDescription != nil {
// deleting the listener before recreating it
t.Cloud.ELB().DeleteLoadBalancerListeners(&elb.DeleteLoadBalancerListenersInput{
LoadBalancerName: aws.String(loadBalancerName),
LoadBalancerPorts: []*int64{aws.Int64(443)},
})
}

request := &elb.CreateLoadBalancerListenersInput{}
request.LoadBalancerName = aws.String(loadBalancerName)

Expand All @@ -572,7 +572,7 @@ func (_ *LoadBalancer) RenderAWS(t *awsup.AWSAPITarget, a, e, changes *LoadBalan

glog.V(2).Infof("Creating LoadBalancer listeners")

_, err := t.Cloud.ELB().CreateLoadBalancerListeners(request)
_, err = t.Cloud.ELB().CreateLoadBalancerListeners(request)
if err != nil {
return fmt.Errorf("error creating LoadBalancerListeners: %v", err)
}
Expand Down