Skip to content

Commit dfda5ac

Browse files
committed
feedback addressed
1 parent 32e804a commit dfda5ac

File tree

4 files changed

+31
-88
lines changed

4 files changed

+31
-88
lines changed

go.mod

+3-3
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ require (
77
github.com/PuerkitoBio/goquery v1.10.1
88
github.com/avast/retry-go v3.0.0+incompatible
99
github.com/aws/amazon-vpc-resource-controller-k8s v1.6.3
10-
github.com/aws/aws-sdk-go-v2 v1.36.0
10+
github.com/aws/aws-sdk-go-v2 v1.36.1
1111
github.com/aws/aws-sdk-go-v2/config v1.29.4
1212
github.com/aws/aws-sdk-go-v2/feature/ec2/imds v1.16.27
1313
github.com/aws/aws-sdk-go-v2/service/ec2 v1.202.2
@@ -52,8 +52,8 @@ require (
5252
github.com/Masterminds/semver/v3 v3.2.1 // indirect
5353
github.com/andybalholm/cascadia v1.3.3 // indirect
5454
github.com/aws/aws-sdk-go-v2/credentials v1.17.57 // indirect
55-
github.com/aws/aws-sdk-go-v2/internal/configsources v1.3.31 // indirect
56-
github.com/aws/aws-sdk-go-v2/internal/endpoints/v2 v2.6.31 // indirect
55+
github.com/aws/aws-sdk-go-v2/internal/configsources v1.3.32 // indirect
56+
github.com/aws/aws-sdk-go-v2/internal/endpoints/v2 v2.6.32 // indirect
5757
github.com/aws/aws-sdk-go-v2/internal/ini v1.8.2 // indirect
5858
github.com/aws/aws-sdk-go-v2/service/internal/accept-encoding v1.12.2 // indirect
5959
github.com/aws/aws-sdk-go-v2/service/internal/endpoint-discovery v1.10.12 // indirect

go.sum

+6-6
Original file line numberDiff line numberDiff line change
@@ -10,18 +10,18 @@ github.com/avast/retry-go v3.0.0+incompatible h1:4SOWQ7Qs+oroOTQOYnAHqelpCO0biHS
1010
github.com/avast/retry-go v3.0.0+incompatible/go.mod h1:XtSnn+n/sHqQIpZ10K1qAevBhOOCWBLXXy3hyiqqBrY=
1111
github.com/aws/amazon-vpc-resource-controller-k8s v1.6.3 h1:B4o15iZP8CQoyDjoNAoQiyEPabLsgxXLY5tv3uvvCic=
1212
github.com/aws/amazon-vpc-resource-controller-k8s v1.6.3/go.mod h1:k4zcf2Dz/Mvrgo8NVzAEWP5HK4USqbJTD93pVVDxvc0=
13-
github.com/aws/aws-sdk-go-v2 v1.36.0 h1:b1wM5CcE65Ujwn565qcwgtOTT1aT4ADOHHgglKjG7fk=
14-
github.com/aws/aws-sdk-go-v2 v1.36.0/go.mod h1:5PMILGVKiW32oDzjj6RU52yrNrDPUHcbZQYr1sM7qmM=
13+
github.com/aws/aws-sdk-go-v2 v1.36.1 h1:iTDl5U6oAhkNPba0e1t1hrwAo02ZMqbrGq4k5JBWM5E=
14+
github.com/aws/aws-sdk-go-v2 v1.36.1/go.mod h1:5PMILGVKiW32oDzjj6RU52yrNrDPUHcbZQYr1sM7qmM=
1515
github.com/aws/aws-sdk-go-v2/config v1.29.4 h1:ObNqKsDYFGr2WxnoXKOhCvTlf3HhwtoGgc+KmZ4H5yg=
1616
github.com/aws/aws-sdk-go-v2/config v1.29.4/go.mod h1:j2/AF7j/qxVmsNIChw1tWfsVKOayJoGRDjg1Tgq7NPk=
1717
github.com/aws/aws-sdk-go-v2/credentials v1.17.57 h1:kFQDsbdBAR3GZsB8xA+51ptEnq9TIj3tS4MuP5b+TcQ=
1818
github.com/aws/aws-sdk-go-v2/credentials v1.17.57/go.mod h1:2kerxPUUbTagAr/kkaHiqvj/bcYHzi2qiJS/ZinllU0=
1919
github.com/aws/aws-sdk-go-v2/feature/ec2/imds v1.16.27 h1:7lOW8NUwE9UZekS1DYoiPdVAqZ6A+LheHWb+mHbNOq8=
2020
github.com/aws/aws-sdk-go-v2/feature/ec2/imds v1.16.27/go.mod h1:w1BASFIPOPUae7AgaH4SbjNbfdkxuggLyGfNFTn8ITY=
21-
github.com/aws/aws-sdk-go-v2/internal/configsources v1.3.31 h1:lWm9ucLSRFiI4dQQafLrEOmEDGry3Swrz0BIRdiHJqQ=
22-
github.com/aws/aws-sdk-go-v2/internal/configsources v1.3.31/go.mod h1:Huu6GG0YTfbPphQkDSo4dEGmQRTKb9k9G7RdtyQWxuI=
23-
github.com/aws/aws-sdk-go-v2/internal/endpoints/v2 v2.6.31 h1:ACxDklUKKXb48+eg5ROZXi1vDgfMyfIA/WyvqHcHI0o=
24-
github.com/aws/aws-sdk-go-v2/internal/endpoints/v2 v2.6.31/go.mod h1:yadnfsDwqXeVaohbGc/RaD287PuyRw2wugkh5ZL2J6k=
21+
github.com/aws/aws-sdk-go-v2/internal/configsources v1.3.32 h1:BjUcr3X3K0wZPGFg2bxOWW3VPN8rkE3/61zhP+IHviA=
22+
github.com/aws/aws-sdk-go-v2/internal/configsources v1.3.32/go.mod h1:80+OGC/bgzzFFTUmcuwD0lb4YutwQeKLFpmt6hoWapU=
23+
github.com/aws/aws-sdk-go-v2/internal/endpoints/v2 v2.6.32 h1:m1GeXHVMJsRsUAqG6HjZWx9dj7F5TR+cF1bjyfYyBd4=
24+
github.com/aws/aws-sdk-go-v2/internal/endpoints/v2 v2.6.32/go.mod h1:IitoQxGfaKdVLNg0hD8/DXmAqNy0H4K2H2Sf91ti8sI=
2525
github.com/aws/aws-sdk-go-v2/internal/ini v1.8.2 h1:Pg9URiobXy85kgFev3og2CuOZ8JZUBENF+dcgWBaYNk=
2626
github.com/aws/aws-sdk-go-v2/internal/ini v1.8.2/go.mod h1:FbtygfRFze9usAadmnGJNc8KsP346kEe+y2/oyhGAGc=
2727
github.com/aws/aws-sdk-go-v2/service/ec2 v1.202.2 h1:qas57zkkMX8OM+MVz+4sMaOaD9HRmeFJRb8nzMdYkx0=

pkg/controllers/nodeclass/validation.go

+22-19
Original file line numberDiff line numberDiff line change
@@ -82,23 +82,25 @@ func (n Validation) Reconcile(ctx context.Context, nodeClass *v1.EC2NodeClass) (
8282
createFleetInput.DryRun = aws.Bool(true)
8383

8484
if _, err := n.ec2api.CreateFleet(ctx, createFleetInput); awserrors.IgnoreDryRunError(err) != nil {
85-
nodeClass.StatusConditions().SetFalse(v1.ConditionTypeValidationSucceeded, "CreateFleetAuthCheckFailed", "Controller isn't authorized to call CreateFleet")
8685
if awserrors.IgnoreUnauthorizedOperationError(err) != nil {
86+
// Dry run should only ever return UnauthorizedOperation or DryRunOperation so if we receive any other error
87+
// it would be an unexpected state
8788
return reconcile.Result{}, fmt.Errorf("unexpected error during CreateFleet validation: %w", err)
8889
}
90+
nodeClass.StatusConditions().SetFalse(v1.ConditionTypeValidationSucceeded, "CreateFleetAuthCheckFailed", "Controller isn't authorized to call CreateFleet")
8991
return reconcile.Result{}, nil
9092
}
9193

9294
createLaunchTemplateInput := launchtemplate.GetCreateLaunchTemplateInput(mockOptions(*nodeClaim, nodeClass, tags), corev1.IPv4Protocol, "")
9395
createLaunchTemplateInput.DryRun = aws.Bool(true)
9496

9597
if _, err := n.ec2api.CreateLaunchTemplate(ctx, createLaunchTemplateInput); awserrors.IgnoreDryRunError(err) != nil {
96-
nodeClass.StatusConditions().SetFalse(v1.ConditionTypeValidationSucceeded, "CreateLaunchTemplateAuthCheckFailed", "Controller isn't authorized to call CreateLaunchTemplate")
9798
if awserrors.IgnoreUnauthorizedOperationError(err) != nil {
9899
// Dry run should only ever return UnauthorizedOperation or DryRunOperation so if we receive any other error
99100
// it would be an unexpected state
100101
return reconcile.Result{}, fmt.Errorf("unexpected error during CreateLaunchTemplate validation: %w", err)
101102
}
103+
nodeClass.StatusConditions().SetFalse(v1.ConditionTypeValidationSucceeded, "CreateLaunchTemplateAuthCheckFailed", "Controller isn't authorized to call CreateLaunchTemplate")
102104
return reconcile.Result{}, nil
103105
}
104106

@@ -108,19 +110,18 @@ func (n Validation) Reconcile(ctx context.Context, nodeClass *v1.EC2NodeClass) (
108110
}
109111

110112
var instanceType ec2types.InstanceType
111-
if len(nodeClass.Status.AMIs) > 0 {
112-
requirements := scheduling.NewRequirements(lo.Map(nodeClass.Status.AMIs[0].Requirements, func(req corev1.NodeSelectorRequirement, _ int) *scheduling.Requirement {
113-
return scheduling.NewRequirement(req.Key, req.Operator, req.Values...)
114-
})...)
115-
116-
if arch := requirements.Get("kubernetes.io/arch"); len(arch.Values()) > 0 {
117-
switch arch.Values()[0] {
118-
case "amd64":
119-
instanceType = ec2types.InstanceTypeM5Large
120-
case "arm64":
121-
instanceType = ec2types.InstanceTypeM6gLarge
122-
}
113+
requirements := scheduling.NewNodeSelectorRequirements(lo.Map(nodeClass.Status.AMIs[0].Requirements, func(req corev1.NodeSelectorRequirement, _ int) corev1.NodeSelectorRequirement {
114+
return corev1.NodeSelectorRequirement{
115+
Key: req.Key,
116+
Operator: req.Operator,
117+
Values: req.Values,
123118
}
119+
})...)
120+
121+
if requirements.Get(corev1.LabelArchStable).Has(karpv1.ArchitectureAmd64) {
122+
instanceType = ec2types.InstanceTypeM5Large
123+
} else if requirements.Get(corev1.LabelArchStable).Has(karpv1.ArchitectureArm64) {
124+
instanceType = ec2types.InstanceTypeM6gLarge
124125
}
125126

126127
runInstancesInput := &ec2.RunInstancesInput{
@@ -129,9 +130,11 @@ func (n Validation) Reconcile(ctx context.Context, nodeClass *v1.EC2NodeClass) (
129130
MinCount: aws.Int32(1),
130131
InstanceType: instanceType,
131132
MetadataOptions: &ec2types.InstanceMetadataOptionsRequest{
132-
HttpTokens: ec2types.HttpTokensStateRequired,
133-
HttpEndpoint: ec2types.InstanceMetadataEndpointStateDisabled,
134-
HttpPutResponseHopLimit: aws.Int32(2),
133+
HttpEndpoint: ec2types.InstanceMetadataEndpointState(lo.FromPtr(nodeClass.Spec.MetadataOptions.HTTPEndpoint)),
134+
HttpTokens: ec2types.HttpTokensState(lo.FromPtr(nodeClass.Spec.MetadataOptions.HTTPTokens)),
135+
//aws sdk v2 changed this type to *int32 instead of *int64
136+
//nolint: gosec
137+
HttpPutResponseHopLimit: aws.Int32(int32(lo.FromPtr(nodeClass.Spec.MetadataOptions.HTTPPutResponseHopLimit))),
135138
},
136139
TagSpecifications: []ec2types.TagSpecification{
137140
{
@@ -151,12 +154,12 @@ func (n Validation) Reconcile(ctx context.Context, nodeClass *v1.EC2NodeClass) (
151154
}
152155

153156
if _, err = n.ec2api.RunInstances(ctx, runInstancesInput); awserrors.IgnoreDryRunError(err) != nil {
154-
nodeClass.StatusConditions().SetFalse(v1.ConditionTypeValidationSucceeded, "RunInstancesAuthCheckFailed", "Controller isn't authorized to call RunInstances")
155157
if awserrors.IgnoreUnauthorizedOperationError(err) != nil {
156158
// Dry run should only ever return UnauthorizedOperation or DryRunOperation so if we receive any other error
157159
// it would be an unexpected state
158160
return reconcile.Result{}, fmt.Errorf("unexpected error during RunInstances validation: %w", err)
159161
}
162+
nodeClass.StatusConditions().SetFalse(v1.ConditionTypeValidationSucceeded, "RunInstancesAuthCheckFailed", "Controller isn't authorized to call RunInstances")
160163
return reconcile.Result{}, nil
161164
}
162165
nodeClass.StatusConditions().SetTrue(v1.ConditionTypeValidationSucceeded)
@@ -195,7 +198,7 @@ func mockOptions(nodeClaim karpv1.NodeClaim, nodeClass *v1.EC2NodeClass, tags ma
195198
HTTPEndpoint: nodeClass.Spec.MetadataOptions.HTTPEndpoint,
196199
HTTPTokens: nodeClass.Spec.MetadataOptions.HTTPTokens,
197200
HTTPProtocolIPv6: nodeClass.Spec.MetadataOptions.HTTPProtocolIPv6,
198-
HTTPPutResponseHopLimit: aws.Int64(1),
201+
HTTPPutResponseHopLimit: nodeClass.Spec.MetadataOptions.HTTPPutResponseHopLimit,
199202
},
200203
AMIID: nodeClaim.Status.ImageID,
201204
BlockDeviceMappings: nodeClass.Spec.BlockDeviceMappings,

test/suites/integration/nodeclass_test.go

-60
Original file line numberDiff line numberDiff line change
@@ -111,64 +111,4 @@ var _ = Describe("NodeClass IAM Permissions", func() {
111111
g.Expect(nodeClass.StatusConditions().Get(v1.ConditionTypeValidationSucceeded).IsTrue()).To(BeTrue())
112112
}, "60s", "5s").Should(Succeed())
113113
})
114-
115-
It("Service Control Policy Tests",
116-
func() {
117-
policyDoc := fmt.Sprintf(`{
118-
"Version": "2012-10-17",
119-
"Statement": [
120-
{
121-
"Effect": "Deny",
122-
"Action": ["ec2:RunInstances"],
123-
"Resource": "*",
124-
"Condition": {
125-
"StringNotLike": {
126-
"%s": ["%s"]
127-
}
128-
}
129-
}
130-
]
131-
}`, v1.LabelNodeClass, nodeClass.Name)
132-
deployment := &appsv1.Deployment{}
133-
err := env.Client.Get(env.Context, types.NamespacedName{
134-
Namespace: "kube-system",
135-
Name: "karpenter",
136-
}, deployment)
137-
Expect(err).To(BeNil())
138-
139-
sa := &corev1.ServiceAccount{}
140-
err = env.Client.Get(env.Context, types.NamespacedName{
141-
Namespace: "kube-system",
142-
Name: deployment.Spec.Template.Spec.ServiceAccountName,
143-
}, sa)
144-
Expect(err).To(BeNil())
145-
146-
roleName = strings.Split(sa.Annotations["eks.amazonaws.com/role-arn"], "/")[1]
147-
policyName = fmt.Sprintf("TestSCP-%s", uuid.New().String())
148-
149-
// Attach the test SCP-like policy
150-
_, err = env.IAMAPI.PutRolePolicy(env.Context, &iam.PutRolePolicyInput{
151-
RoleName: aws.String(roleName),
152-
PolicyName: aws.String(policyName),
153-
PolicyDocument: aws.String(policyDoc),
154-
})
155-
Expect(err).To(BeNil())
156-
157-
DeferCleanup(func() {
158-
_, err := env.IAMAPI.DeleteRolePolicy(env.Context, &iam.DeleteRolePolicyInput{
159-
RoleName: aws.String(roleName),
160-
PolicyName: aws.String(policyName),
161-
})
162-
Expect(err).To(BeNil())
163-
})
164-
165-
env.ExpectCreated(nodeClass)
166-
Eventually(func(g Gomega) {
167-
env.ExpectUpdated(nodeClass)
168-
g.Expect(nodeClass.StatusConditions().Get(v1.ConditionTypeValidationSucceeded).IsFalse()).To(BeTrue())
169-
g.Expect(nodeClass.StatusConditions().Get(v1.ConditionTypeValidationSucceeded).Reason).To(Equal("RunInstancesAuthCheckFailed"))
170-
}, "240s", "5s").Should(Succeed())
171-
ExpectStatusConditions(env, env.Client, 1*time.Minute, nodeClass, status.Condition{Type: status.ConditionReady, Status: metav1.ConditionFalse, Message: "ValidationSucceeded=False"})
172-
},
173-
)
174114
})

0 commit comments

Comments
 (0)