Skip to content

Commit

Permalink
Imporve handling of *bool and *string fileds
Browse files Browse the repository at this point in the history 
- renane `NewBoolTrue` & `NewBoolFalse` to `Enabled` `Disabled`
- add `IsEnabled` and `IsDisabled`
- add `IsSetAndNonEmptyString`
  • Loading branch information
@errordeveloper
errordeveloper committed Apr 9, 2019
1 parent 05c5784 commit 209413b
Show file tree
Hide file tree
Showing 15 changed files with 104 additions and 94 deletions.
24 changes: 12 additions & 12 deletions pkg/apis/eksctl.io/v1alpha4/defaults.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,25 +18,25 @@ func SetNodeGroupDefaults(_ int, ng *NodeGroup) error {
}
}
if ng.SecurityGroups.WithLocal == nil {
ng.SecurityGroups.WithLocal = NewBoolTrue()
ng.SecurityGroups.WithLocal = Enabled()
}
if ng.SecurityGroups.WithShared == nil {
ng.SecurityGroups.WithShared = NewBoolTrue()
ng.SecurityGroups.WithShared = Enabled()
}

if ng.SSH == nil {
ng.SSH = &NodeGroupSSH{
Allow: NewBoolFalse(),
Allow: Disabled(),
}
}

if ng.SSH.Allow == nil {
ng.SSH.Allow = NewBoolFalse()
ng.SSH.Allow = Disabled()
}

// Enable SSH when a key is provided
if ng.SSH.PublicKeyPath != nil {
ng.SSH.Allow = NewBoolTrue()
ng.SSH.Allow = Enabled()
}

if *ng.SSH.Allow && ng.SSH.PublicKeyPath == nil {
Expand All @@ -53,25 +53,25 @@ func SetNodeGroupDefaults(_ int, ng *NodeGroup) error {
ng.IAM = &NodeGroupIAM{}
}
if ng.IAM.WithAddonPolicies.ImageBuilder == nil {
ng.IAM.WithAddonPolicies.ImageBuilder = NewBoolFalse()
ng.IAM.WithAddonPolicies.ImageBuilder = Disabled()
}
if ng.IAM.WithAddonPolicies.AutoScaler == nil {
ng.IAM.WithAddonPolicies.AutoScaler = NewBoolFalse()
ng.IAM.WithAddonPolicies.AutoScaler = Disabled()
}
if ng.IAM.WithAddonPolicies.ExternalDNS == nil {
ng.IAM.WithAddonPolicies.ExternalDNS = NewBoolFalse()
ng.IAM.WithAddonPolicies.ExternalDNS = Disabled()
}
if ng.IAM.WithAddonPolicies.ALBIngress == nil {
ng.IAM.WithAddonPolicies.ALBIngress = NewBoolFalse()
ng.IAM.WithAddonPolicies.ALBIngress = Disabled()
}
if ng.IAM.WithAddonPolicies.EBS == nil {
ng.IAM.WithAddonPolicies.EBS = NewBoolFalse()
ng.IAM.WithAddonPolicies.EBS = Disabled()
}
if ng.IAM.WithAddonPolicies.FSX == nil {
ng.IAM.WithAddonPolicies.FSX = NewBoolFalse()
ng.IAM.WithAddonPolicies.FSX = Disabled()
}
if ng.IAM.WithAddonPolicies.EFS == nil {
ng.IAM.WithAddonPolicies.EFS = NewBoolFalse()
ng.IAM.WithAddonPolicies.EFS = Disabled()
}

return nil
Expand Down
4 changes: 2 additions & 2 deletions pkg/apis/eksctl.io/v1alpha4/defaults_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ var _ = Describe("Default settings", func() {
It("Providing an SSH key enables SSH", func() {
testNodeGroup := NodeGroup{
SSH: &NodeGroupSSH{
Allow: NewBoolFalse(),
Allow: Disabled(),
PublicKeyPath: &testKeyPath,
},
}
Expand All @@ -28,7 +28,7 @@ var _ = Describe("Default settings", func() {
It("Enabling SSH without a key uses default key", func() {
testNodeGroup := NodeGroup{
SSH: &NodeGroupSSH{
Allow: NewBoolTrue(),
Allow: Enabled(),
},
}

Expand Down
39 changes: 24 additions & 15 deletions pkg/apis/eksctl.io/v1alpha4/types.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -123,20 +123,29 @@ var (
DefaultNodeSSHPublicKeyPath = "~/.ssh/id_rsa.pub"
)

// NewBoolTrue return pointer to true value
// Enabled return pointer to true value
// for use in defaulters of *bool fields
func NewBoolTrue() *bool {
func Enabled() *bool {
v := true
return &v
}

// NewBoolFalse return pointer to false value
// Disabled return pointer to false value
// for use in defaulters of *bool fields
func NewBoolFalse() *bool {
func Disabled() *bool {
v := false
return &v
}

// IsEnabled will only return true if v is not nil and true
func IsEnabled(v *bool) bool { return v != nil && *v }

// IsDisabled will only return true if v is not nil and false
func IsDisabled(v *bool) bool { return v != nil && !*v }

// IsSetAndNonEmptyString will only return true if s is not nil and not empty
func IsSetAndNonEmptyString(s *string) bool { return s != nil && *s != "" }

// SupportedRegions are the regions where EKS is available
func SupportedRegions() []string {
return []string{
Expand Down Expand Up @@ -317,27 +326,27 @@ func (c *ClusterConfig) NewNodeGroup() *NodeGroup {
PrivateNetworking: false,
SecurityGroups: &NodeGroupSGs{
AttachIDs: []string{},
WithLocal: NewBoolTrue(),
WithShared: NewBoolTrue(),
WithLocal: Enabled(),
WithShared: Enabled(),
},
DesiredCapacity: nil,
InstanceType: DefaultNodeType,
VolumeSize: 0,
VolumeType: DefaultNodeVolumeType,
IAM: &NodeGroupIAM{
WithAddonPolicies: NodeGroupIAMAddonPolicies{
ImageBuilder: NewBoolFalse(),
AutoScaler: NewBoolFalse(),
ExternalDNS: NewBoolFalse(),
AppMesh: NewBoolFalse(),
EBS: NewBoolFalse(),
FSX: NewBoolFalse(),
EFS: NewBoolFalse(),
ALBIngress: NewBoolFalse(),
ImageBuilder: Disabled(),
AutoScaler: Disabled(),
ExternalDNS: Disabled(),
AppMesh: Disabled(),
EBS: Disabled(),
FSX: Disabled(),
EFS: Disabled(),
ALBIngress: Disabled(),
},
},
SSH: &NodeGroupSSH{
Allow: NewBoolFalse(),
Allow: Disabled(),
PublicKeyPath: &DefaultNodeSSHPublicKeyPath,
},
}
Expand Down
16 changes: 8 additions & 8 deletions pkg/apis/eksctl.io/v1alpha4/validation.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,28 +16,28 @@ func validateNodeGroupIAM(i int, ng *NodeGroup, value, fieldName, path string) e
if len(ng.IAM.AttachPolicyARNs) != 0 {
return fmt.Errorf("%s.attachPolicyARNs cannot be set at the same time", p)
}
if v := ng.IAM.WithAddonPolicies.AutoScaler; v != nil && *v {
if IsEnabled(ng.IAM.WithAddonPolicies.AutoScaler) {
return fmt.Errorf("%s.withAddonPolicies.autoScaler cannot be set at the same time", p)
}
if v := ng.IAM.WithAddonPolicies.ExternalDNS; v != nil && *v {
if IsEnabled(ng.IAM.WithAddonPolicies.ExternalDNS) {
return fmt.Errorf("%s.withAddonPolicies.externalDNS cannot be set at the same time", p)
}
if v := ng.IAM.WithAddonPolicies.ImageBuilder; v != nil && *v {
if IsEnabled(ng.IAM.WithAddonPolicies.ImageBuilder) {
return fmt.Errorf("%s.imageBuilder cannot be set at the same time", p)
}
if v := ng.IAM.WithAddonPolicies.AppMesh; v != nil && *v {
if IsEnabled(ng.IAM.WithAddonPolicies.AppMesh) {
return fmt.Errorf("%s.AppMesh cannot be set at the same time", p)
}
if v := ng.IAM.WithAddonPolicies.EBS; v != nil && *v {
if IsEnabled(ng.IAM.WithAddonPolicies.EBS) {
return fmt.Errorf("%s.ebs cannot be set at the same time", p)
}
if v := ng.IAM.WithAddonPolicies.FSX; v != nil && *v {
if IsEnabled(ng.IAM.WithAddonPolicies.FSX) {
return fmt.Errorf("%s.fsx cannot be set at the same time", p)
}
if v := ng.IAM.WithAddonPolicies.EFS; v != nil && *v {
if IsEnabled(ng.IAM.WithAddonPolicies.EFS) {
return fmt.Errorf("%s.efs cannot be set at the same time", p)
}
if v := ng.IAM.WithAddonPolicies.ALBIngress; v != nil && *v {
if IsEnabled(ng.IAM.WithAddonPolicies.ALBIngress) {
return fmt.Errorf("%s.albIngress cannot be set at the same time", p)
}
}
Expand Down
44 changes: 22 additions & 22 deletions pkg/cfn/builder/api_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -330,27 +330,27 @@ var _ = Describe("CloudFormation template builder API", func() {
Name: "ng-abcd1234",
PrivateNetworking: false,
SecurityGroups: &api.NodeGroupSGs{
WithLocal: api.NewBoolTrue(),
WithShared: api.NewBoolTrue(),
WithLocal: api.Enabled(),
WithShared: api.Enabled(),
AttachIDs: []string{},
},
DesiredCapacity: nil,
VolumeSize: 2,
VolumeType: api.NodeVolumeTypeIO1,
IAM: &api.NodeGroupIAM{
WithAddonPolicies: api.NodeGroupIAMAddonPolicies{
ImageBuilder: api.NewBoolFalse(),
AutoScaler: api.NewBoolFalse(),
ExternalDNS: api.NewBoolFalse(),
AppMesh: api.NewBoolFalse(),
EBS: api.NewBoolFalse(),
FSX: api.NewBoolFalse(),
EFS: api.NewBoolFalse(),
ALBIngress: api.NewBoolFalse(),
ImageBuilder: api.Disabled(),
AutoScaler: api.Disabled(),
ExternalDNS: api.Disabled(),
AppMesh: api.Disabled(),
EBS: api.Disabled(),
FSX: api.Disabled(),
EFS: api.Disabled(),
ALBIngress: api.Disabled(),
},
},
SSH: &api.NodeGroupSSH{
Allow: api.NewBoolFalse(),
Allow: api.Disabled(),
PublicKeyPath: &defaultSSHKeyPath,
},
},
Expand Down Expand Up @@ -475,7 +475,7 @@ var _ = Describe("CloudFormation template builder API", func() {
Context("NodeGroupAutoScaling", func() {
cfg, ng := newClusterConfigAndNodegroup(true)

ng.IAM.WithAddonPolicies.AutoScaler = api.NewBoolTrue()
ng.IAM.WithAddonPolicies.AutoScaler = api.Enabled()

build(cfg, "eksctl-test-123-cluster", ng)

Expand Down Expand Up @@ -533,8 +533,8 @@ var _ = Describe("CloudFormation template builder API", func() {
Context("NodeGroupAppMeshExternalDNS", func() {
cfg, ng := newClusterConfigAndNodegroup(true)

ng.IAM.WithAddonPolicies.AppMesh = api.NewBoolTrue()
ng.IAM.WithAddonPolicies.ExternalDNS = api.NewBoolTrue()
ng.IAM.WithAddonPolicies.AppMesh = api.Enabled()
ng.IAM.WithAddonPolicies.ExternalDNS = api.Enabled()

build(cfg, "eksctl-test-megaapps-cluster", ng)

Expand Down Expand Up @@ -577,7 +577,7 @@ var _ = Describe("CloudFormation template builder API", func() {
Context("NodeGroupALBIngress", func() {
cfg, ng := newClusterConfigAndNodegroup(true)

ng.IAM.WithAddonPolicies.ALBIngress = api.NewBoolTrue()
ng.IAM.WithAddonPolicies.ALBIngress = api.Enabled()

build(cfg, "eksctl-test-megaapps-cluster", ng)

Expand Down Expand Up @@ -664,7 +664,7 @@ var _ = Describe("CloudFormation template builder API", func() {
cfg, ng := newClusterConfigAndNodegroup(true)

ng.VolumeSize = 0
ng.IAM.WithAddonPolicies.EBS = api.NewBoolTrue()
ng.IAM.WithAddonPolicies.EBS = api.Enabled()

build(cfg, "eksctl-test-ebs-cluster", ng)

Expand Down Expand Up @@ -706,7 +706,7 @@ var _ = Describe("CloudFormation template builder API", func() {
cfg, ng := newClusterConfigAndNodegroup(true)

ng.VolumeSize = 0
ng.IAM.WithAddonPolicies.FSX = api.NewBoolTrue()
ng.IAM.WithAddonPolicies.FSX = api.Enabled()

build(cfg, "eksctl-test-fsx-cluster", ng)

Expand All @@ -732,12 +732,12 @@ var _ = Describe("CloudFormation template builder API", func() {
Expect(obj.Resources).ToNot(HaveKey("PolicyAppMesh"))
})
})

Context("NodeGroupEFS", func() {
cfg, ng := newClusterConfigAndNodegroup(true)

ng.VolumeSize = 0
ng.IAM.WithAddonPolicies.EFS = api.NewBoolTrue()
ng.IAM.WithAddonPolicies.EFS = api.Enabled()

build(cfg, "eksctl-test-efs-cluster", ng)

Expand Down Expand Up @@ -767,7 +767,7 @@ var _ = Describe("CloudFormation template builder API", func() {
Context("NodeGroup{PrivateNetworking=true SSH.Allow=true}", func() {
cfg, ng := newClusterConfigAndNodegroup(true)

ng.SSH.Allow = api.NewBoolTrue()
ng.SSH.Allow = api.Enabled()
keyName := ""
ng.SSH.PublicKeyName = &keyName
ng.InstanceType = "t2.medium"
Expand Down Expand Up @@ -825,7 +825,7 @@ var _ = Describe("CloudFormation template builder API", func() {
Context("NodeGroup{PrivateNetworking=false SSH.Allow=true}", func() {
cfg, ng := newClusterConfigAndNodegroup(true)

ng.SSH.Allow = api.NewBoolTrue()
ng.SSH.Allow = api.Enabled()
keyName := ""
ng.SSH.PublicKeyName = &keyName
ng.InstanceType = "t2.medium"
Expand Down Expand Up @@ -906,7 +906,7 @@ var _ = Describe("CloudFormation template builder API", func() {
}

ng.AvailabilityZones = []string{testAZs[1]}
ng.SSH.Allow = api.NewBoolFalse()
ng.SSH.Allow = api.Disabled()
ng.InstanceType = "t2.medium"
ng.PrivateNetworking = false
ng.AMIFamily = "AmazonLinux2"
Expand Down
Loading

0 comments on commit 209413b

Please sign in to comment.