From 86f157fa271e1e83cc14f1fc106acb57670800bb Mon Sep 17 00:00:00 2001 From: John Gardiner Myers Date: Fri, 26 Jun 2020 20:52:30 -0700 Subject: [PATCH] Refactor how api-server addresses are exported from tasks --- pkg/model/alimodel/BUILD.bazel | 1 - pkg/model/alimodel/api_loadbalancer.go | 9 +--- pkg/model/awsmodel/BUILD.bazel | 1 - pkg/model/awsmodel/api_loadbalancer.go | 14 ++---- pkg/model/domodel/BUILD.bazel | 1 - pkg/model/domodel/api_loadbalancer.go | 10 +--- pkg/model/gcemodel/BUILD.bazel | 1 - pkg/model/gcemodel/api_loadbalancer.go | 9 +--- pkg/model/openstackmodel/BUILD.bazel | 1 - pkg/model/openstackmodel/servergroup.go | 31 ++++--------- upup/pkg/fi/cloudup/alitasks/loadbalancer.go | 5 ++ upup/pkg/fi/cloudup/awstasks/elastic_ip.go | 13 ------ upup/pkg/fi/cloudup/awstasks/load_balancer.go | 7 ++- upup/pkg/fi/cloudup/dotasks/loadbalancer.go | 12 +++-- upup/pkg/fi/cloudup/gcetasks/address.go | 7 ++- .../fi/cloudup/openstacktasks/floatingip.go | 15 ++++-- .../pkg/fi/cloudup/openstacktasks/instance.go | 7 ++- upup/pkg/fi/fitasks/keypair.go | 46 +++++++++++++------ upup/pkg/fi/fitasks/keypair_test.go | 5 +- upup/pkg/fi/has_address.go | 9 ++-- 20 files changed, 97 insertions(+), 107 deletions(-) diff --git a/pkg/model/alimodel/BUILD.bazel b/pkg/model/alimodel/BUILD.bazel index f3f36d323a..cbc4fab7b3 100644 --- a/pkg/model/alimodel/BUILD.bazel +++ b/pkg/model/alimodel/BUILD.bazel @@ -25,7 +25,6 @@ go_library( "//pkg/util/stringorslice:go_default_library", "//upup/pkg/fi:go_default_library", "//upup/pkg/fi/cloudup/alitasks:go_default_library", - "//upup/pkg/fi/fitasks:go_default_library", "//util/pkg/vfs:go_default_library", "//vendor/github.com/denverdino/aliyungo/ram:go_default_library", "//vendor/k8s.io/klog:go_default_library", diff --git a/pkg/model/alimodel/api_loadbalancer.go b/pkg/model/alimodel/api_loadbalancer.go index 189099f390..dedbb83879 100644 --- a/pkg/model/alimodel/api_loadbalancer.go +++ b/pkg/model/alimodel/api_loadbalancer.go @@ -24,7 +24,6 @@ import ( "k8s.io/kops/pkg/dns" "k8s.io/kops/upup/pkg/fi" "k8s.io/kops/upup/pkg/fi/cloudup/alitasks" - "k8s.io/kops/upup/pkg/fi/fitasks" ) const ( @@ -133,13 +132,7 @@ func (b *APILoadBalancerModelBuilder) Build(c *fi.ModelBuilderContext) error { if dns.IsGossipHostname(b.Cluster.Name) || b.UsePrivateDNS() { // Ensure the ELB hostname is included in the TLS certificate, // if we're not going to use an alias for it - // TODO: I don't love this technique for finding the task by name & modifying it - masterKeypairTask, found := c.Tasks["Keypair/master"] - if !found { - return errors.New("keypair/master task not found") - } - masterKeypair := masterKeypairTask.(*fitasks.Keypair) - masterKeypair.AlternateNameTasks = append(masterKeypair.AlternateNameTasks, loadbalancer) + loadbalancer.ForAPIServer = true } return nil diff --git a/pkg/model/awsmodel/BUILD.bazel b/pkg/model/awsmodel/BUILD.bazel index a67667c2d6..97059adaeb 100644 --- a/pkg/model/awsmodel/BUILD.bazel +++ b/pkg/model/awsmodel/BUILD.bazel @@ -18,7 +18,6 @@ go_library( "//pkg/model/spotinstmodel:go_default_library", "//upup/pkg/fi:go_default_library", "//upup/pkg/fi/cloudup/awstasks:go_default_library", - "//upup/pkg/fi/fitasks:go_default_library", "//vendor/github.com/aws/aws-sdk-go/service/ec2:go_default_library", "//vendor/k8s.io/apimachinery/pkg/util/sets:go_default_library", "//vendor/k8s.io/klog:go_default_library", diff --git a/pkg/model/awsmodel/api_loadbalancer.go b/pkg/model/awsmodel/api_loadbalancer.go index 8357c5c38c..437018e0f2 100644 --- a/pkg/model/awsmodel/api_loadbalancer.go +++ b/pkg/model/awsmodel/api_loadbalancer.go @@ -21,15 +21,13 @@ import ( "sort" "time" + "k8s.io/apimachinery/pkg/util/sets" + "k8s.io/klog" "k8s.io/kops/pkg/apis/kops" "k8s.io/kops/pkg/dns" "k8s.io/kops/pkg/featureflag" "k8s.io/kops/upup/pkg/fi" "k8s.io/kops/upup/pkg/fi/cloudup/awstasks" - "k8s.io/kops/upup/pkg/fi/fitasks" - - "k8s.io/apimachinery/pkg/util/sets" - "k8s.io/klog" ) // LoadBalancerDefaultIdleTimeout is the default idle time for the ELB @@ -272,13 +270,7 @@ func (b *APILoadBalancerBuilder) Build(c *fi.ModelBuilderContext) error { if dns.IsGossipHostname(b.Cluster.Name) || b.UsePrivateDNS() { // Ensure the ELB hostname is included in the TLS certificate, // if we're not going to use an alias for it - // TODO: I don't love this technique for finding the task by name & modifying it - masterKeypairTask, found := c.Tasks["Keypair/master"] - if !found { - return fmt.Errorf("keypair/master task not found") - } - masterKeypair := masterKeypairTask.(*fitasks.Keypair) - masterKeypair.AlternateNameTasks = append(masterKeypair.AlternateNameTasks, elb) + elb.ForAPIServer = true } // When Spotinst Elastigroups are used, there is no need to create diff --git a/pkg/model/domodel/BUILD.bazel b/pkg/model/domodel/BUILD.bazel index c6af73b155..7bbd474e78 100644 --- a/pkg/model/domodel/BUILD.bazel +++ b/pkg/model/domodel/BUILD.bazel @@ -16,6 +16,5 @@ go_library( "//upup/pkg/fi:go_default_library", "//upup/pkg/fi/cloudup/do:go_default_library", "//upup/pkg/fi/cloudup/dotasks:go_default_library", - "//upup/pkg/fi/fitasks:go_default_library", ], ) diff --git a/pkg/model/domodel/api_loadbalancer.go b/pkg/model/domodel/api_loadbalancer.go index 58f578bd33..735181b45e 100644 --- a/pkg/model/domodel/api_loadbalancer.go +++ b/pkg/model/domodel/api_loadbalancer.go @@ -17,7 +17,6 @@ limitations under the License. package domodel import ( - "errors" "fmt" "strings" @@ -26,7 +25,6 @@ import ( "k8s.io/kops/upup/pkg/fi" "k8s.io/kops/upup/pkg/fi/cloudup/do" "k8s.io/kops/upup/pkg/fi/cloudup/dotasks" - "k8s.io/kops/upup/pkg/fi/fitasks" ) // APILoadBalancerModelBuilder builds a LoadBalancer for accessing the API @@ -75,13 +73,7 @@ func (b *APILoadBalancerModelBuilder) Build(c *fi.ModelBuilderContext) error { if dns.IsGossipHostname(b.Cluster.Name) || b.UsePrivateDNS() { // Ensure the LB hostname is included in the TLS certificate, // if we're not going to use an alias for it - // TODO: I don't love this technique for finding the task by name & modifying it - masterKeypairTask, found := c.Tasks["Keypair/master"] - if !found { - return errors.New("keypair/master task not found") - } - masterKeypair := masterKeypairTask.(*fitasks.Keypair) - masterKeypair.AlternateNameTasks = append(masterKeypair.AlternateNameTasks, loadbalancer) + loadbalancer.ForAPIServer = true } return nil diff --git a/pkg/model/gcemodel/BUILD.bazel b/pkg/model/gcemodel/BUILD.bazel index 3ce85fc390..e32a094c92 100644 --- a/pkg/model/gcemodel/BUILD.bazel +++ b/pkg/model/gcemodel/BUILD.bazel @@ -25,7 +25,6 @@ go_library( "//upup/pkg/fi:go_default_library", "//upup/pkg/fi/cloudup/gce:go_default_library", "//upup/pkg/fi/cloudup/gcetasks:go_default_library", - "//upup/pkg/fi/fitasks:go_default_library", "//util/pkg/vfs:go_default_library", "//vendor/k8s.io/apimachinery/pkg/util/sets:go_default_library", "//vendor/k8s.io/klog:go_default_library", diff --git a/pkg/model/gcemodel/api_loadbalancer.go b/pkg/model/gcemodel/api_loadbalancer.go index 76b63525e4..72d6c36b47 100644 --- a/pkg/model/gcemodel/api_loadbalancer.go +++ b/pkg/model/gcemodel/api_loadbalancer.go @@ -22,7 +22,6 @@ import ( "k8s.io/kops/pkg/apis/kops" "k8s.io/kops/upup/pkg/fi" "k8s.io/kops/upup/pkg/fi/cloudup/gcetasks" - "k8s.io/kops/upup/pkg/fi/fitasks" ) // APILoadBalancerBuilder builds a LoadBalancer for accessing the API @@ -78,13 +77,7 @@ func (b *APILoadBalancerBuilder) Build(c *fi.ModelBuilderContext) error { { // Ensure the IP address is included in our certificate - // TODO: I don't love this technique for finding the task by name & modifying it - masterKeypairTask, found := c.Tasks["Keypair/master"] - if !found { - return fmt.Errorf("keypair/master task not found") - } - masterKeypair := masterKeypairTask.(*fitasks.Keypair) - masterKeypair.AlternateNameTasks = append(masterKeypair.AlternateNameTasks, ipAddress) + ipAddress.ForAPIServer = true } // Allow traffic into the API (port 443) from KubernetesAPIAccess CIDRs diff --git a/pkg/model/openstackmodel/BUILD.bazel b/pkg/model/openstackmodel/BUILD.bazel index cbd592987d..be1ae3df96 100644 --- a/pkg/model/openstackmodel/BUILD.bazel +++ b/pkg/model/openstackmodel/BUILD.bazel @@ -20,7 +20,6 @@ go_library( "//upup/pkg/fi:go_default_library", "//upup/pkg/fi/cloudup/openstack:go_default_library", "//upup/pkg/fi/cloudup/openstacktasks:go_default_library", - "//upup/pkg/fi/fitasks:go_default_library", "//vendor/github.com/gophercloud/gophercloud/openstack/networking/v2/extensions/security/rules:go_default_library", "//vendor/k8s.io/cloud-provider-openstack/pkg/util/openstack:go_default_library", "//vendor/k8s.io/klog:go_default_library", diff --git a/pkg/model/openstackmodel/servergroup.go b/pkg/model/openstackmodel/servergroup.go index 65047f1953..fd1c7172cc 100644 --- a/pkg/model/openstackmodel/servergroup.go +++ b/pkg/model/openstackmodel/servergroup.go @@ -27,7 +27,6 @@ import ( "k8s.io/kops/upup/pkg/fi" "k8s.io/kops/upup/pkg/fi/cloudup/openstack" "k8s.io/kops/upup/pkg/fi/cloudup/openstacktasks" - "k8s.io/kops/upup/pkg/fi/fitasks" ) // ServerGroupModelBuilder configures server group objects @@ -163,7 +162,7 @@ func (b *ServerGroupModelBuilder) buildInstances(c *fi.ModelBuilderContext, sg * if b.Cluster.Spec.CloudConfig.Openstack != nil && b.Cluster.Spec.CloudConfig.Openstack.Router != nil { if ig.Spec.AssociatePublicIP != nil && !fi.BoolValue(ig.Spec.AssociatePublicIP) { if ig.Spec.Role == kops.InstanceGroupRoleMaster { - b.associateFixedIPToKeypair(c, instanceTask) + b.associateFixedIPToKeypair(instanceTask) } continue } @@ -183,7 +182,7 @@ func (b *ServerGroupModelBuilder) buildInstances(c *fi.ModelBuilderContext, sg * Lifecycle: b.Lifecycle, } c.AddTask(t) - b.associateFIPToKeypair(c, t) + b.associateFIPToKeypair(t) } default: if !b.UsesSSHBastion() { @@ -198,7 +197,7 @@ func (b *ServerGroupModelBuilder) buildInstances(c *fi.ModelBuilderContext, sg * } else if b.Cluster.Spec.CloudConfig.Openstack != nil && b.Cluster.Spec.CloudConfig.Openstack.Router == nil { // No external router, but we need to add master fixed ips to certificates if ig.Spec.Role == kops.InstanceGroupRoleMaster { - b.associateFixedIPToKeypair(c, instanceTask) + b.associateFixedIPToKeypair(instanceTask) } } } @@ -206,30 +205,16 @@ func (b *ServerGroupModelBuilder) buildInstances(c *fi.ModelBuilderContext, sg * return nil } -func (b *ServerGroupModelBuilder) associateFixedIPToKeypair(c *fi.ModelBuilderContext, fipTask *openstacktasks.Instance) error { +func (b *ServerGroupModelBuilder) associateFixedIPToKeypair(fipTask *openstacktasks.Instance) { // Ensure the floating IP is included in the TLS certificate, // if we're not going to use an alias for it - // TODO: I don't love this technique for finding the task by name & modifying it - masterKeypairTask, found := c.Tasks["Keypair/master"] - if !found { - return fmt.Errorf("keypair/master task not found") - } - masterKeypair := masterKeypairTask.(*fitasks.Keypair) - masterKeypair.AlternateNameTasks = append(masterKeypair.AlternateNameTasks, fipTask) - return nil + fipTask.ForAPIServer = true } -func (b *ServerGroupModelBuilder) associateFIPToKeypair(c *fi.ModelBuilderContext, fipTask *openstacktasks.FloatingIP) error { +func (b *ServerGroupModelBuilder) associateFIPToKeypair(fipTask *openstacktasks.FloatingIP) { // Ensure the floating IP is included in the TLS certificate, // if we're not going to use an alias for it - // TODO: I don't love this technique for finding the task by name & modifying it - masterKeypairTask, found := c.Tasks["Keypair/master"] - if !found { - return fmt.Errorf("keypair/master task not found") - } - masterKeypair := masterKeypairTask.(*fitasks.Keypair) - masterKeypair.AlternateNameTasks = append(masterKeypair.AlternateNameTasks, fipTask) - return nil + fipTask.ForAPIServer = true } func (b *ServerGroupModelBuilder) Build(c *fi.ModelBuilderContext) error { @@ -294,7 +279,7 @@ func (b *ServerGroupModelBuilder) Build(c *fi.ModelBuilderContext) error { c.AddTask(lbfipTask) if dns.IsGossipHostname(b.Cluster.Name) || b.UsePrivateDNS() { - b.associateFIPToKeypair(c, lbfipTask) + b.associateFIPToKeypair(lbfipTask) } poolTask := &openstacktasks.LBPool{ diff --git a/upup/pkg/fi/cloudup/alitasks/loadbalancer.go b/upup/pkg/fi/cloudup/alitasks/loadbalancer.go index d0172932ea..acafe82881 100644 --- a/upup/pkg/fi/cloudup/alitasks/loadbalancer.go +++ b/upup/pkg/fi/cloudup/alitasks/loadbalancer.go @@ -40,6 +40,7 @@ type LoadBalancer struct { LoadBalancerAddress *string Lifecycle *fi.Lifecycle Tags map[string]string + ForAPIServer bool } var _ fi.CompareWithID = &LoadBalancer{} @@ -107,6 +108,10 @@ func (l *LoadBalancer) Find(c *fi.Context) (*LoadBalancer, error) { return actual, nil } +func (l *LoadBalancer) IsForAPIServer() bool { + return l.ForAPIServer +} + func (l *LoadBalancer) FindIPAddress(context *fi.Context) (*string, error) { cloud := context.Cloud.(aliup.ALICloud) diff --git a/upup/pkg/fi/cloudup/awstasks/elastic_ip.go b/upup/pkg/fi/cloudup/awstasks/elastic_ip.go index 0a0491d5bf..56ae839cfa 100644 --- a/upup/pkg/fi/cloudup/awstasks/elastic_ip.go +++ b/upup/pkg/fi/cloudup/awstasks/elastic_ip.go @@ -56,19 +56,6 @@ func (e *ElasticIP) CompareWithID() *string { return e.ID } -var _ fi.HasAddress = &ElasticIP{} - -func (e *ElasticIP) FindIPAddress(context *fi.Context) (*string, error) { - actual, err := e.find(context.Cloud.(awsup.AWSCloud)) - if err != nil { - return nil, fmt.Errorf("error querying for ElasticIP: %v", err) - } - if actual == nil { - return nil, nil - } - return actual.PublicIP, nil -} - // Find returns the actual ElasticIP state, or nil if not found func (e *ElasticIP) Find(context *fi.Context) (*ElasticIP, error) { return e.find(context.Cloud.(awsup.AWSCloud)) diff --git a/upup/pkg/fi/cloudup/awstasks/load_balancer.go b/upup/pkg/fi/cloudup/awstasks/load_balancer.go index 3b09a3b0f8..32dd88b02b 100644 --- a/upup/pkg/fi/cloudup/awstasks/load_balancer.go +++ b/upup/pkg/fi/cloudup/awstasks/load_balancer.go @@ -65,7 +65,8 @@ type LoadBalancer struct { CrossZoneLoadBalancing *LoadBalancerCrossZoneLoadBalancing SSLCertificateID string - Tags map[string]string + Tags map[string]string + ForAPIServer bool } var _ fi.CompareWithID = &LoadBalancer{} @@ -405,6 +406,10 @@ func (e *LoadBalancer) Find(c *fi.Context) (*LoadBalancer, error) { var _ fi.HasAddress = &LoadBalancer{} +func (e *LoadBalancer) IsForAPIServer() bool { + return e.ForAPIServer +} + func (e *LoadBalancer) FindIPAddress(context *fi.Context) (*string, error) { cloud := context.Cloud.(awsup.AWSCloud) diff --git a/upup/pkg/fi/cloudup/dotasks/loadbalancer.go b/upup/pkg/fi/cloudup/dotasks/loadbalancer.go index f7ed3e2de5..94353c810f 100644 --- a/upup/pkg/fi/cloudup/dotasks/loadbalancer.go +++ b/upup/pkg/fi/cloudup/dotasks/loadbalancer.go @@ -38,12 +38,14 @@ type LoadBalancer struct { ID *string Lifecycle *fi.Lifecycle - Region *string - DropletTag *string - IPAddress *string + Region *string + DropletTag *string + IPAddress *string + ForAPIServer bool } var _ fi.CompareWithID = &LoadBalancer{} +var _ fi.HasAddress = &LoadBalancer{} func (lb *LoadBalancer) CompareWithID() *string { return lb.ID @@ -165,6 +167,10 @@ func (_ *LoadBalancer) RenderDO(t *do.DOAPITarget, a, e, changes *LoadBalancer) return nil } +func (lb *LoadBalancer) IsForAPIServer() bool { + return lb.ForAPIServer +} + func (lb *LoadBalancer) FindIPAddress(c *fi.Context) (*string, error) { cloud := c.Cloud.(*digitalocean.Cloud) loadBalancerService := cloud.LoadBalancers() diff --git a/upup/pkg/fi/cloudup/gcetasks/address.go b/upup/pkg/fi/cloudup/gcetasks/address.go index 9203a466e0..593dea1c33 100644 --- a/upup/pkg/fi/cloudup/gcetasks/address.go +++ b/upup/pkg/fi/cloudup/gcetasks/address.go @@ -31,7 +31,8 @@ type Address struct { Name *string Lifecycle *fi.Lifecycle - IPAddress *string + IPAddress *string + ForAPIServer bool } func (e *Address) Find(c *fi.Context) (*Address, error) { @@ -84,6 +85,10 @@ func (e *Address) find(cloud gce.GCECloud) (*Address, error) { var _ fi.HasAddress = &Address{} +func (e *Address) IsForAPIServer() bool { + return e.ForAPIServer +} + func (e *Address) FindIPAddress(context *fi.Context) (*string, error) { actual, err := e.find(context.Cloud.(gce.GCECloud)) if err != nil { diff --git a/upup/pkg/fi/cloudup/openstacktasks/floatingip.go b/upup/pkg/fi/cloudup/openstacktasks/floatingip.go index a69c1502d7..48689db404 100644 --- a/upup/pkg/fi/cloudup/openstacktasks/floatingip.go +++ b/upup/pkg/fi/cloudup/openstacktasks/floatingip.go @@ -31,11 +31,12 @@ import ( //go:generate fitask -type=FloatingIP type FloatingIP struct { - Name *string - ID *string - Server *Instance - LB *LB - Lifecycle *fi.Lifecycle + Name *string + ID *string + Server *Instance + LB *LB + Lifecycle *fi.Lifecycle + ForAPIServer bool } var _ fi.HasAddress = &FloatingIP{} @@ -82,6 +83,10 @@ func (e *FloatingIP) findServerFloatingIP(context *fi.Context, cloud openstack.O return nil, nil } +func (e *FloatingIP) IsForAPIServer() bool { + return e.ForAPIServer +} + func (e *FloatingIP) FindIPAddress(context *fi.Context) (*string, error) { if e.ID == nil { if e.Server != nil && e.Server.ID == nil { diff --git a/upup/pkg/fi/cloudup/openstacktasks/instance.go b/upup/pkg/fi/cloudup/openstacktasks/instance.go index 8f1b20ff67..d915da54a0 100644 --- a/upup/pkg/fi/cloudup/openstacktasks/instance.go +++ b/upup/pkg/fi/cloudup/openstacktasks/instance.go @@ -46,7 +46,8 @@ type Instance struct { AvailabilityZone *string SecurityGroups []string - Lifecycle *fi.Lifecycle + Lifecycle *fi.Lifecycle + ForAPIServer bool } var _ fi.HasAddress = &Instance{} @@ -75,6 +76,10 @@ func (e *Instance) CompareWithID() *string { return e.ID } +func (e *Instance) IsForAPIServer() bool { + return e.ForAPIServer +} + func (e *Instance) FindIPAddress(context *fi.Context) (*string, error) { cloud := context.Cloud.(openstack.OpenstackCloud) if e.Port == nil { diff --git a/upup/pkg/fi/fitasks/keypair.go b/upup/pkg/fi/fitasks/keypair.go index 0f26415238..52209cae87 100644 --- a/upup/pkg/fi/fitasks/keypair.go +++ b/upup/pkg/fi/fitasks/keypair.go @@ -35,7 +35,7 @@ type Keypair struct { // AlternateNames a list of alternative names for this certificate AlternateNames []string `json:"alternateNames"` // AlternateNameTasks is a collection of subtask - AlternateNameTasks []fi.Task `json:"alternateNameTasks"` + AlternateNameTasks []fi.HasAddress `json:"alternateNameTasks"` // Lifecycle is context for a task Lifecycle *fi.Lifecycle // Signer is the keypair to use to sign, for when we want to use an alternative CA @@ -50,6 +50,7 @@ type Keypair struct { var _ fi.HasCheckExisting = &Keypair{} var _ fi.HasName = &Keypair{} +var _ fi.HasDependencies = &Keypair{} // It's important always to check for the existing key, so we don't regenerate keys e.g. on terraform func (e *Keypair) CheckExisting(c *fi.Context) bool { @@ -62,6 +63,25 @@ func (e *Keypair) CompareWithID() *string { return &e.Subject } +func (e *Keypair) GetDependencies(tasks map[string]fi.Task) []fi.Task { + var deps []fi.Task + + if e.Signer != nil { + deps = append(deps, e.Signer) + } + + if *e.Name == "master" { + for _, task := range tasks { + if hasAddress, ok := task.(fi.HasAddress); ok && hasAddress.IsForAPIServer() { + deps = append(deps, task) + e.AlternateNameTasks = append(e.AlternateNameTasks, hasAddress) + } + } + } + + return deps +} + func (e *Keypair) Find(c *fi.Context) (*Keypair, error) { name := fi.StringValue(e.Name) if name == "" { @@ -122,21 +142,17 @@ func (e *Keypair) normalize(c *fi.Context) error { alternateNames = append(alternateNames, s) } - for _, task := range e.AlternateNameTasks { - if hasAddress, ok := task.(fi.HasAddress); ok { - address, err := hasAddress.FindIPAddress(c) - if err != nil { - return fmt.Errorf("error finding address for %v: %v", task, err) - } - if address == nil { - klog.Warningf("Task did not have an address: %v", task) - continue - } - klog.V(8).Infof("Resolved alternateName %q for %q", *address, task) - alternateNames = append(alternateNames, *address) - } else { - return fmt.Errorf("Unsupported type for AlternateNameDependencies: %v", task) + for _, hasAddress := range e.AlternateNameTasks { + address, err := hasAddress.FindIPAddress(c) + if err != nil { + return fmt.Errorf("error finding address for %v: %v", hasAddress, err) } + if address == nil { + klog.Warningf("Task did not have an address: %v", hasAddress) + continue + } + klog.V(8).Infof("Resolved alternateName %q for %q", *address, hasAddress) + alternateNames = append(alternateNames, *address) } sort.Strings(alternateNames) diff --git a/upup/pkg/fi/fitasks/keypair_test.go b/upup/pkg/fi/fitasks/keypair_test.go index d90347dc18..71d1b2baa4 100644 --- a/upup/pkg/fi/fitasks/keypair_test.go +++ b/upup/pkg/fi/fitasks/keypair_test.go @@ -24,8 +24,11 @@ import ( ) func TestKeypairDeps(t *testing.T) { - ca := &Keypair{} + ca := &Keypair{ + Name: fi.String("ca"), + } cert := &Keypair{ + Name: fi.String("cert"), Signer: ca, } diff --git a/upup/pkg/fi/has_address.go b/upup/pkg/fi/has_address.go index 26d208bff1..f31f4b24f5 100644 --- a/upup/pkg/fi/has_address.go +++ b/upup/pkg/fi/has_address.go @@ -16,9 +16,12 @@ limitations under the License. package fi -// HasAddress is implemented by elastic/floating IP addresses, to expose the address -// For example, this is used so that the master SSL certificate can be configured with the dynamically allocated IP +// HasAddress is implemented by elastic/floating IP addresses in order to include +// relevant dynamically allocated addresses in the api-server's server TLS certificate. type HasAddress interface { - // FindIPAddress returns the address associated with the implementor. If there is no address, returns (nil, nil) + Task + // IsForAPIServer indicates whether the implementation provides an address that needs to be added to the api-server server certificate. + IsForAPIServer() bool + // FindIPAddress returns the address associated with the implementor. If there is no address, returns (nil, nil). FindIPAddress(context *Context) (*string, error) }