Refactor how api-server addresses are exported from tasks

This commit is contained in:
John Gardiner Myers 2020-06-26 20:52:30 -07:00
parent 07dc255559
commit 86f157fa27
20 changed files with 97 additions and 107 deletions

View File

@ -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",

View File

@ -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

View File

@ -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",

View File

@ -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

View File

@ -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",
],
)

View File

@ -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

View File

@ -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",

View File

@ -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

View File

@ -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",

View File

@ -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{

View File

@ -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)

View File

@ -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))

View File

@ -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)

View File

@ -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()

View File

@ -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 {

View File

@ -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 {

View File

@ -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 {

View File

@ -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)

View File

@ -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,
}

View File

@ -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)
}