Skip to content

Commit

Permalink
AWS always uses resource-based names
Browse files Browse the repository at this point in the history
  • Loading branch information
johngmyers committed Sep 4, 2023
1 parent 9ced296 commit 1ea0fd3
Show file tree
Hide file tree
Showing 15 changed files with 16 additions and 73 deletions.
3 changes: 0 additions & 3 deletions cmd/kops-controller/pkg/config/options.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
2 changes: 1 addition & 1 deletion cmd/kops-controller/pkg/server/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
2 changes: 1 addition & 1 deletion nodeup/pkg/model/kubelet.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
10 changes: 0 additions & 10 deletions pkg/apis/nodeup/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"`

Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -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 {
Expand Down
2 changes: 1 addition & 1 deletion pkg/bootstrap/authenticate.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
6 changes: 2 additions & 4 deletions upup/pkg/fi/cloudup/awsup/aws_cloud.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
}
Expand All @@ -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)
Expand Down
4 changes: 2 additions & 2 deletions upup/pkg/fi/cloudup/awsup/aws_verifier.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
}
Expand Down Expand Up @@ -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
}
Expand Down
2 changes: 1 addition & 1 deletion upup/pkg/fi/cloudup/azure/verifier.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
}
Expand Down
2 changes: 1 addition & 1 deletion upup/pkg/fi/cloudup/do/verifier.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
}
Expand Down
2 changes: 1 addition & 1 deletion upup/pkg/fi/cloudup/gce/tpm/gcetpmverifier/tpmverifier.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.

Expand Down
2 changes: 1 addition & 1 deletion upup/pkg/fi/cloudup/hetzner/verifier.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
}
Expand Down
2 changes: 1 addition & 1 deletion upup/pkg/fi/cloudup/openstack/verifier.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
}
Expand Down
2 changes: 1 addition & 1 deletion upup/pkg/fi/cloudup/scaleway/verifier.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
}
Expand Down
7 changes: 0 additions & 7 deletions upup/pkg/fi/cloudup/template_functions.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -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)

Expand Down
41 changes: 3 additions & 38 deletions upup/pkg/fi/nodeup/command.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
}
Expand All @@ -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)
Expand Down

0 comments on commit 1ea0fd3

Please sign in to comment.