Skip to content

Commit

Permalink
Apply suggestions from code review
Browse files Browse the repository at this point in the history
Co-authored-by: Ciprian Hacman <ciprianhacman@gmail.com>
  • Loading branch information
bharath-123 and hakman authored Dec 3, 2020
1 parent 8ce54a2 commit edb1cc0
Show file tree
Hide file tree
Showing 7 changed files with 21 additions and 25 deletions.
4 changes: 2 additions & 2 deletions pkg/apis/kops/instancegroup.go
Original file line number Diff line number Diff line change
Expand Up @@ -185,8 +185,8 @@ type InstanceMetadataOptions struct {
// HTTPPutResponseHopLimit is the desired HTTP PUT response hop limit for instance metadata requests.
// The larger the number, the further instance metadata requests can travel. The default value is 1.
HTTPPutResponseHopLimit *int64 `json:"httpPutResponseHopLimit,omitempty"`
// HTTPTokens can be either 'required' or 'optional'. If value is 'required', then EC2 IMDv2 will be enabled and
// EC2 IMDv1 will be disabled. If it is 'optional', both v1 and v2 will be enabled (default 'optional')
// HTTPTokens is the state of token usage for the instance metadata requests.
// If the parameter is not specified in the request, the default state is "optional".
HTTPTokens *string `json:"httpTokens,omitempty"`
}

Expand Down
20 changes: 8 additions & 12 deletions pkg/apis/kops/validation/aws.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,20 +66,16 @@ func awsValidateInstanceGroup(ig *kops.InstanceGroup, cloud awsup.AWSCloud) fiel
func awsValidateInstanceMetadata(fieldPath *field.Path, instanceMetadata *kops.InstanceMetadataOptions) field.ErrorList {
allErrs := field.ErrorList{}

httpTokens := fi.StringValue(instanceMetadata.HTTPTokens)
httpPutResponseLimit := fi.Int64Value(instanceMetadata.HTTPPutResponseHopLimit)

// we do not allow disabling InstanceMetadata since some k8s and kops components require InstanceMetadata
if httpTokens != "required" && httpTokens != "optional" {
allErrs = append(allErrs, field.Invalid(fieldPath.Child("httpTokens"), instanceMetadata.HTTPTokens,
"Invalid HTTPTokens value. Possible values are 'required' or 'optional'. See https://docs.aws.amazon.com/cli/latest/reference/ec2/modify-instance-metadata-options.html"+
" for more details"))
if instanceMetadata.HTTPTokens != nil {
allErrs = append(allErrs, IsValidValue(fieldPath.Child("httpTokens"), instanceMetadata.HTTPTokens, []string{"optional", "required"})...)
}

if httpPutResponseLimit <= 0 || httpPutResponseLimit > 64 {
allErrs = append(allErrs, field.Invalid(fieldPath.Child("httpPutResponseLimit"), instanceMetadata.HTTPPutResponseHopLimit,
"HTTPPutResponseLimit must be a value between 1 and 64. See https://docs.aws.amazon.com/cli/latest/reference/ec2/modify-instance-metadata-options.html"+
" for more details"))
if instanceMetadata.HTTPPutResponseHopLimit != nil {
httpPutResponseHopLimit := fi.Int64Value(instanceMetadata.HTTPPutResponseHopLimit)
if httpPutResponseHopLimit < 1 || httpPutResponseHopLimit > 64 {
allErrs = append(allErrs, field.Invalid(fieldPath.Child("httpPutResponseHopLimit"), instanceMetadata.HTTPPutResponseHopLimit,
"HTTPPutResponseLimit must be a value between 1 and 64"))
}
}

return allErrs
Expand Down
4 changes: 2 additions & 2 deletions pkg/apis/kops/validation/instancegroup_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -335,7 +335,7 @@ func TestInstanceMetadataOptions(t *testing.T) {
},
},
cloud: awsup.BuildMockAWSCloud("us-east-1", "abc"),
expected: []string{"Invalid value::spec.instanceMetadata.httpTokens"},
expected: []string{"Unsupported value::spec.instanceMetadata.httpTokens"},
},
{
ig: &kops.InstanceGroup{
Expand All @@ -351,7 +351,7 @@ func TestInstanceMetadataOptions(t *testing.T) {
},
},
cloud: awsup.BuildMockAWSCloud("us-east-1", "abc"),
expected: []string{"Invalid value::spec.instanceMetadata.httpPutResponseLimit"},
expected: []string{"Invalid value::spec.instanceMetadata.httpPutResponseHopLimit"},
},
}

Expand Down
4 changes: 2 additions & 2 deletions upup/pkg/fi/cloudup/awstasks/launchtemplate.go
Original file line number Diff line number Diff line change
Expand Up @@ -74,9 +74,9 @@ type LaunchTemplate struct {
Tenancy *string
// UserData is the user data configuration
UserData *fi.ResourceHolder
// HTTPTokens is an enum which if set to "required" ensure that EC2 IMDv1 is disabled and only IMDv2 is used
// HTTPTokens is the state of token usage for your instance metadata requests.
HTTPTokens *string
// HTTPPutResponseHopLimit is the max no of hops made by the PUT request which fetches the token (default '1')
// HTTPPutResponseHopLimit is the desired HTTP PUT response hop limit for instance metadata requests.
HTTPPutResponseHopLimit *int64
}

Expand Down
2 changes: 1 addition & 1 deletion upup/pkg/fi/cloudup/awstasks/launchtemplate_target_api.go
Original file line number Diff line number Diff line change
Expand Up @@ -276,7 +276,7 @@ func (t *LaunchTemplate) Find(c *fi.Context) (*LaunchTemplate, error) {
actual.UserData = fi.WrapResource(fi.NewStringResource(string(ud)))
}

// @step: Add instance metadata related options
// @step: add instance metadata options
if lt.LaunchTemplateData.MetadataOptions != nil {
actual.HTTPPutResponseHopLimit = lt.LaunchTemplateData.MetadataOptions.HttpPutResponseHopLimit
actual.HTTPTokens = lt.LaunchTemplateData.MetadataOptions.HttpTokens
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -109,9 +109,9 @@ type cloudformationLaunchTemplateTagSpecification struct {
}

type cloudformationLaunchTemplateInstanceMetadataOptions struct {
// HTTPTokens determines whether the metadata service requires a token or not
// HTTPTokens is the state of token usage for your instance metadata requests.
HTTPTokens *string `json:"HttpTokens,omitempty"`
// HTTPPutResponseHopLimit determines the desired HTTP PUT response hop limit for instance metadata requests.
// HTTPPutResponseHopLimit is the desired HTTP PUT response hop limit for instance metadata requests.
HTTPPutResponseHopLimit *int64 `json:"HttpPutResponseHopLimit,omitempty"`
}

Expand Down Expand Up @@ -140,7 +140,7 @@ type cloudformationLaunchTemplateData struct {
TagSpecifications []*cloudformationLaunchTemplateTagSpecification `json:"TagSpecifications,omitempty"`
// UserData is the user data for the instances
UserData *string `json:"UserData,omitempty"`
// MetadataOptions contains the fields to configure the instance metadata of the ec2 instance
// MetadataOptions are the instance metadata options.
MetadataOptions *cloudformationLaunchTemplateInstanceMetadataOptions `json:"MetadataOptions,omitempty"`
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -109,9 +109,9 @@ type terraformLaunchTemplateTagSpecification struct {
}

type terraformLaunchTemplateInstanceMetadata struct {
// HTTPTokens determines whether the metadata service requires a token or not
// HTTPTokens is the state of token usage for your instance metadata requests.
HTTPTokens *string `json:"http_tokens,omitempty" cty:"http_tokens"`
// HTTPPutResponseHopLimit determines the desired HTTP PUT response hop limit for instance metadata requests.
// HTTPPutResponseHopLimit is the desired HTTP PUT response hop limit for instance metadata requests.
HTTPPutResponseHopLimit *int64 `json:"http_put_response_hop_limit,omitempty" cty:"http_put_response_hop_limit"`
}

Expand Down Expand Up @@ -147,7 +147,7 @@ type terraformLaunchTemplate struct {
TagSpecifications []*terraformLaunchTemplateTagSpecification `json:"tag_specifications,omitempty" cty:"tag_specifications"`
// UserData is the user data for the instances
UserData *terraform.Literal `json:"user_data,omitempty" cty:"user_data"`
// MetadataOptions contains the fields to configure the instance metadata of the ec2 instance
// MetadataOptions are the instance metadata options.
MetadataOptions *terraformLaunchTemplateInstanceMetadata `json:"metadata_options,omitempty" cty:"metadata_options"`
}

Expand Down

0 comments on commit edb1cc0

Please sign in to comment.