From 1ea0fd30045a6adcb5e213a7688e70593b0263ef Mon Sep 17 00:00:00 2001 From: John Gardiner Myers Date: Mon, 4 Sep 2023 16:06:46 -0700 Subject: [PATCH] AWS always uses resource-based names --- cmd/kops-controller/pkg/config/options.go | 3 -- cmd/kops-controller/pkg/server/server.go | 2 +- nodeup/pkg/model/kubelet.go | 2 +- pkg/apis/nodeup/config.go | 10 ----- pkg/bootstrap/authenticate.go | 2 +- upup/pkg/fi/cloudup/awsup/aws_cloud.go | 6 +-- upup/pkg/fi/cloudup/awsup/aws_verifier.go | 4 +- upup/pkg/fi/cloudup/azure/verifier.go | 2 +- upup/pkg/fi/cloudup/do/verifier.go | 2 +- .../gce/tpm/gcetpmverifier/tpmverifier.go | 2 +- upup/pkg/fi/cloudup/hetzner/verifier.go | 2 +- upup/pkg/fi/cloudup/openstack/verifier.go | 2 +- upup/pkg/fi/cloudup/scaleway/verifier.go | 2 +- upup/pkg/fi/cloudup/template_functions.go | 7 ---- upup/pkg/fi/nodeup/command.go | 41 ++----------------- 15 files changed, 16 insertions(+), 73 deletions(-) diff --git a/cmd/kops-controller/pkg/config/options.go b/cmd/kops-controller/pkg/config/options.go index 7527a765f44fd..39770c12315e2 100644 --- a/cmd/kops-controller/pkg/config/options.go +++ b/cmd/kops-controller/pkg/config/options.go @@ -62,9 +62,6 @@ type ServerOptions struct { SigningCAs []string `json:"signingCAs"` // CertNames is the list of active certificate names. CertNames []string `json:"certNames"` - - // UseInstanceIDForNodeName uses the instance ID instead of the hostname for the node name. - UseInstanceIDForNodeName bool `json:"useInstanceIDForNodeName,omitempty"` } type ServerProviderOptions struct { diff --git a/cmd/kops-controller/pkg/server/server.go b/cmd/kops-controller/pkg/server/server.go index 07ded09a08946..f5579d4d48fa9 100644 --- a/cmd/kops-controller/pkg/server/server.go +++ b/cmd/kops-controller/pkg/server/server.go @@ -157,7 +157,7 @@ func (s *Server) bootstrap(w http.ResponseWriter, r *http.Request) { ctx := r.Context() - id, err := s.verifier.VerifyToken(ctx, r, r.Header.Get("Authorization"), body, s.opt.Server.UseInstanceIDForNodeName) + id, err := s.verifier.VerifyToken(ctx, r, r.Header.Get("Authorization"), body) if err != nil { // means that we should exit nodeup gracefully if err == bootstrap.ErrAlreadyExists { diff --git a/nodeup/pkg/model/kubelet.go b/nodeup/pkg/model/kubelet.go index 060a32613396c..99cb9c1c7d14a 100644 --- a/nodeup/pkg/model/kubelet.go +++ b/nodeup/pkg/model/kubelet.go @@ -726,7 +726,7 @@ func (b *KubeletBuilder) kubeletNames() ([]string, error) { return nil, fmt.Errorf("error describing instances: %v", err) } - return awsup.GetInstanceCertificateNames(result, b.NodeupConfig.UseInstanceIDForNodeName) + return awsup.GetInstanceCertificateNames(result) } func (b *KubeletBuilder) buildCgroupService(name string) *nodetasks.Service { diff --git a/pkg/apis/nodeup/config.go b/pkg/apis/nodeup/config.go index 460d762cb81fb..b203a202343da 100644 --- a/pkg/apis/nodeup/config.go +++ b/pkg/apis/nodeup/config.go @@ -116,8 +116,6 @@ type Config struct { ElbSecurityGroup *string `json:"elbSecurityGroup,omitempty"` // NodeIPFamilies controls the IP families reported for each node. NodeIPFamilies []string `json:"nodeIPFamilies,omitempty"` - // UseInstanceIDForNodeName uses the instance ID instead of the hostname for the node name. - UseInstanceIDForNodeName bool `json:"useInstanceIDForNodeName,omitempty"` // WarmPoolImages are the container images to pre-pull during instance pre-initialization WarmPoolImages []string `json:"warmPoolImages,omitempty"` @@ -336,10 +334,6 @@ func NewConfig(cluster *kops.Cluster, instanceGroup *kops.InstanceGroup) (*Confi config.Networking.KubeRouter = &kops.KuberouterNetworkingSpec{} } - if UsesInstanceIDForNodeName(cluster) { - config.UseInstanceIDForNodeName = true - } - if instanceGroup.Spec.Kubelet != nil { config.KubeletConfig = *instanceGroup.Spec.Kubelet } @@ -454,10 +448,6 @@ func buildKubeProxy(cluster *kops.Cluster, instanceGroup *kops.InstanceGroup) *k return config } -func UsesInstanceIDForNodeName(cluster *kops.Cluster) bool { - return cluster.Spec.GetCloudProvider() == kops.CloudProviderAWS -} - func filterFileAssets(f []kops.FileAssetSpec, role kops.InstanceGroupRole) []kops.FileAssetSpec { var fileAssets []kops.FileAssetSpec for _, fileAsset := range f { diff --git a/pkg/bootstrap/authenticate.go b/pkg/bootstrap/authenticate.go index 8f17648ab477a..648cbaa8b9d78 100644 --- a/pkg/bootstrap/authenticate.go +++ b/pkg/bootstrap/authenticate.go @@ -49,5 +49,5 @@ type VerifyResult struct { // Verifier verifies authentication credentials for requests. type Verifier interface { - VerifyToken(ctx context.Context, rawRequest *http.Request, token string, body []byte, useInstanceIDForNodeName bool) (*VerifyResult, error) + VerifyToken(ctx context.Context, rawRequest *http.Request, token string, body []byte) (*VerifyResult, error) } diff --git a/upup/pkg/fi/cloudup/awsup/aws_cloud.go b/upup/pkg/fi/cloudup/awsup/aws_cloud.go index aa35833720a45..97587f3c428af 100644 --- a/upup/pkg/fi/cloudup/awsup/aws_cloud.go +++ b/upup/pkg/fi/cloudup/awsup/aws_cloud.go @@ -2506,7 +2506,7 @@ func GetRolesInInstanceProfile(c AWSCloud, profileName string) ([]string, error) // GetInstanceCertificateNames returns the instance hostname and addresses that should go into certificates. // The first value is the node name and any additional values are the DNS name and IP addresses. -func GetInstanceCertificateNames(instances *ec2.DescribeInstancesOutput, useInstanceIDForNodeName bool) (addrs []string, err error) { +func GetInstanceCertificateNames(instances *ec2.DescribeInstancesOutput) (addrs []string, err error) { if len(instances.Reservations) != 1 { return nil, fmt.Errorf("too many reservations returned for the single instance-id") } @@ -2517,9 +2517,7 @@ func GetInstanceCertificateNames(instances *ec2.DescribeInstancesOutput, useInst instance := instances.Reservations[0].Instances[0] - if useInstanceIDForNodeName { - addrs = append(addrs, *instance.InstanceId) - } + addrs = append(addrs, *instance.InstanceId) if instance.PrivateDnsName != nil { addrs = append(addrs, *instance.PrivateDnsName) diff --git a/upup/pkg/fi/cloudup/awsup/aws_verifier.go b/upup/pkg/fi/cloudup/awsup/aws_verifier.go index 79833b4bfe420..8b98010c0089c 100644 --- a/upup/pkg/fi/cloudup/awsup/aws_verifier.go +++ b/upup/pkg/fi/cloudup/awsup/aws_verifier.go @@ -121,7 +121,7 @@ type ResponseMetadata struct { RequestId string `xml:"RequestId"` } -func (a awsVerifier) VerifyToken(ctx context.Context, rawRequest *http.Request, token string, body []byte, useInstanceIDForNodeName bool) (*bootstrap.VerifyResult, error) { +func (a awsVerifier) VerifyToken(ctx context.Context, rawRequest *http.Request, token string, body []byte) (*bootstrap.VerifyResult, error) { if !strings.HasPrefix(token, AWSAuthenticationTokenPrefix) { return nil, fmt.Errorf("incorrect authorization type") } @@ -233,7 +233,7 @@ func (a awsVerifier) VerifyToken(ctx context.Context, rawRequest *http.Request, instance := instances.Reservations[0].Instances[0] - addrs, err := GetInstanceCertificateNames(instances, useInstanceIDForNodeName) + addrs, err := GetInstanceCertificateNames(instances) if err != nil { return nil, err } diff --git a/upup/pkg/fi/cloudup/azure/verifier.go b/upup/pkg/fi/cloudup/azure/verifier.go index 7f5ccd81888cb..5c853bbf90943 100644 --- a/upup/pkg/fi/cloudup/azure/verifier.go +++ b/upup/pkg/fi/cloudup/azure/verifier.go @@ -58,7 +58,7 @@ func NewAzureVerifier(ctx context.Context, opt *AzureVerifierOptions) (bootstrap }, nil } -func (a azureVerifier) VerifyToken(ctx context.Context, rawRequest *http.Request, token string, body []byte, useInstanceIDForNodeName bool) (*bootstrap.VerifyResult, error) { +func (a azureVerifier) VerifyToken(ctx context.Context, rawRequest *http.Request, token string, body []byte) (*bootstrap.VerifyResult, error) { if !strings.HasPrefix(token, AzureAuthenticationTokenPrefix) { return nil, fmt.Errorf("incorrect authorization type") } diff --git a/upup/pkg/fi/cloudup/do/verifier.go b/upup/pkg/fi/cloudup/do/verifier.go index b5d51cc82f383..d0596a26a8d95 100644 --- a/upup/pkg/fi/cloudup/do/verifier.go +++ b/upup/pkg/fi/cloudup/do/verifier.go @@ -59,7 +59,7 @@ func NewVerifier(ctx context.Context, opt *DigitalOceanVerifierOptions) (bootstr }, nil } -func (o digitalOceanVerifier) VerifyToken(ctx context.Context, rawRequest *http.Request, token string, body []byte, useInstanceIDForNodeName bool) (*bootstrap.VerifyResult, error) { +func (o digitalOceanVerifier) VerifyToken(ctx context.Context, rawRequest *http.Request, token string, body []byte) (*bootstrap.VerifyResult, error) { if !strings.HasPrefix(token, DOAuthenticationTokenPrefix) { return nil, fmt.Errorf("incorrect authorization type") } diff --git a/upup/pkg/fi/cloudup/gce/tpm/gcetpmverifier/tpmverifier.go b/upup/pkg/fi/cloudup/gce/tpm/gcetpmverifier/tpmverifier.go index e32b2ed79bf41..71d30a38785f7 100644 --- a/upup/pkg/fi/cloudup/gce/tpm/gcetpmverifier/tpmverifier.go +++ b/upup/pkg/fi/cloudup/gce/tpm/gcetpmverifier/tpmverifier.go @@ -66,7 +66,7 @@ func NewTPMVerifier(opt *gcetpm.TPMVerifierOptions) (bootstrap.Verifier, error) var _ bootstrap.Verifier = &tpmVerifier{} -func (v *tpmVerifier) VerifyToken(ctx context.Context, rawRequest *http.Request, authToken string, body []byte, useInstanceIDForNodeName bool) (*bootstrap.VerifyResult, error) { +func (v *tpmVerifier) VerifyToken(ctx context.Context, rawRequest *http.Request, authToken string, body []byte) (*bootstrap.VerifyResult, error) { // Reminder: we shouldn't trust any data we get from the client until we've checked the signature (and even then...) // Thankfully the GCE SDK does seem to escape the parameters correctly, for example. diff --git a/upup/pkg/fi/cloudup/hetzner/verifier.go b/upup/pkg/fi/cloudup/hetzner/verifier.go index f01f42d89877f..eb7743684c4f9 100644 --- a/upup/pkg/fi/cloudup/hetzner/verifier.go +++ b/upup/pkg/fi/cloudup/hetzner/verifier.go @@ -57,7 +57,7 @@ func NewHetznerVerifier(opt *HetznerVerifierOptions) (bootstrap.Verifier, error) }, nil } -func (h hetznerVerifier) VerifyToken(ctx context.Context, rawRequest *http.Request, token string, body []byte, useInstanceIDForNodeName bool) (*bootstrap.VerifyResult, error) { +func (h hetznerVerifier) VerifyToken(ctx context.Context, rawRequest *http.Request, token string, body []byte) (*bootstrap.VerifyResult, error) { if !strings.HasPrefix(token, HetznerAuthenticationTokenPrefix) { return nil, fmt.Errorf("incorrect authorization type") } diff --git a/upup/pkg/fi/cloudup/openstack/verifier.go b/upup/pkg/fi/cloudup/openstack/verifier.go index 2e3cbd37657de..0268e162b09f8 100644 --- a/upup/pkg/fi/cloudup/openstack/verifier.go +++ b/upup/pkg/fi/cloudup/openstack/verifier.go @@ -120,7 +120,7 @@ func readKubeConfig() (*restclient.Config, error) { &clientcmd.ConfigOverrides{}).ClientConfig() } -func (o openstackVerifier) VerifyToken(ctx context.Context, rawRequest *http.Request, token string, body []byte, useInstanceIDForNodeName bool) (*bootstrap.VerifyResult, error) { +func (o openstackVerifier) VerifyToken(ctx context.Context, rawRequest *http.Request, token string, body []byte) (*bootstrap.VerifyResult, error) { if !strings.HasPrefix(token, OpenstackAuthenticationTokenPrefix) { return nil, fmt.Errorf("incorrect authorization type") } diff --git a/upup/pkg/fi/cloudup/scaleway/verifier.go b/upup/pkg/fi/cloudup/scaleway/verifier.go index b8cc7a96d231a..a580b6bb516f4 100644 --- a/upup/pkg/fi/cloudup/scaleway/verifier.go +++ b/upup/pkg/fi/cloudup/scaleway/verifier.go @@ -56,7 +56,7 @@ func NewScalewayVerifier(ctx context.Context, opt *ScalewayVerifierOptions) (boo }, nil } -func (v scalewayVerifier) VerifyToken(ctx context.Context, rawRequest *http.Request, token string, body []byte, useInstanceIDForNodeName bool) (*bootstrap.VerifyResult, error) { +func (v scalewayVerifier) VerifyToken(ctx context.Context, rawRequest *http.Request, token string, body []byte) (*bootstrap.VerifyResult, error) { if !strings.HasPrefix(token, ScalewayAuthenticationTokenPrefix) { return nil, fmt.Errorf("incorrect authorization type") } diff --git a/upup/pkg/fi/cloudup/template_functions.go b/upup/pkg/fi/cloudup/template_functions.go index 6c6c6597850b8..d125e918166a3 100644 --- a/upup/pkg/fi/cloudup/template_functions.go +++ b/upup/pkg/fi/cloudup/template_functions.go @@ -50,7 +50,6 @@ import ( "k8s.io/kops/pkg/apis/kops" apiModel "k8s.io/kops/pkg/apis/kops/model" "k8s.io/kops/pkg/apis/kops/util" - "k8s.io/kops/pkg/apis/nodeup" "k8s.io/kops/pkg/dns" "k8s.io/kops/pkg/featureflag" "k8s.io/kops/pkg/flagbuilder" @@ -374,10 +373,6 @@ func (tf *TemplateFunctions) AddTo(dest template.FuncMap, secretStore fi.SecretS dest["ParseTaint"] = util.ParseTaint - dest["UsesInstanceIDForNodeName"] = func() bool { - return nodeup.UsesInstanceIDForNodeName(tf.Cluster) - } - dest["KarpenterEnabled"] = func() bool { return cluster.Spec.Karpenter != nil && cluster.Spec.Karpenter.Enabled } @@ -721,8 +716,6 @@ func (tf *TemplateFunctions) KopsControllerConfig() (string, error) { Region: tf.Region, } - config.Server.UseInstanceIDForNodeName = true - case kops.CloudProviderGCE: c := tf.cloud.(gce.GCECloud) diff --git a/upup/pkg/fi/nodeup/command.go b/upup/pkg/fi/nodeup/command.go index 4885d369dd19f..2c946a0a78acc 100644 --- a/upup/pkg/fi/nodeup/command.go +++ b/upup/pkg/fi/nodeup/command.go @@ -35,7 +35,6 @@ import ( "github.com/aws/aws-sdk-go/aws/ec2metadata" "github.com/aws/aws-sdk-go/aws/session" "github.com/aws/aws-sdk-go/service/autoscaling" - "github.com/aws/aws-sdk-go/service/ec2" "github.com/aws/aws-sdk-go/service/kms" "go.uber.org/multierr" "k8s.io/klog/v2" @@ -421,7 +420,7 @@ func completeWarmingLifecycleAction(cloud awsup.AWSCloud, modelContext *model.No } func evaluateSpec(nodeupConfig *nodeup.Config, cloudProvider api.CloudProviderID) error { - hostnameOverride, err := evaluateHostnameOverride(cloudProvider, nodeupConfig.UseInstanceIDForNodeName) + hostnameOverride, err := evaluateHostnameOverride(cloudProvider) if err != nil { return err } @@ -439,49 +438,15 @@ func evaluateSpec(nodeupConfig *nodeup.Config, cloudProvider api.CloudProviderID return nil } -func evaluateHostnameOverride(cloudProvider api.CloudProviderID, useInstanceIDForNodeName bool) (string, error) { +func evaluateHostnameOverride(cloudProvider api.CloudProviderID) (string, error) { switch cloudProvider { case api.CloudProviderAWS: instanceIDBytes, err := vfs.Context.ReadFile("metadata://aws/meta-data/instance-id") if err != nil { return "", fmt.Errorf("error reading instance-id from AWS metadata: %v", err) } - instanceID := string(instanceIDBytes) - if useInstanceIDForNodeName { - return instanceID, nil - } - - azBytes, err := vfs.Context.ReadFile("metadata://aws/meta-data/placement/availability-zone") - if err != nil { - return "", fmt.Errorf("error reading availability zone from AWS metadata: %v", err) - } - - config := aws.NewConfig() - config = config.WithCredentialsChainVerboseErrors(true) - - s, err := session.NewSession(config) - if err != nil { - return "", fmt.Errorf("error starting new AWS session: %v", err) - } - - svc := ec2.New(s, config.WithRegion(string(azBytes[:len(azBytes)-1]))) - - result, err := svc.DescribeInstances(&ec2.DescribeInstancesInput{ - InstanceIds: []*string{&instanceID}, - }) - if err != nil { - return "", fmt.Errorf("error describing instances: %v", err) - } - - if len(result.Reservations) > 1 { - return "", fmt.Errorf("too many reservations returned for the single instance-id") - } - if len(result.Reservations[0].Instances) > 1 { - return "", fmt.Errorf("too many instances returned for the single instance-id") - } - - return *(result.Reservations[0].Instances[0].PrivateDnsName), nil + return string(instanceIDBytes), nil case api.CloudProviderGCE: // This lets us tolerate broken hostnames (i.e. systemd)