Skip to content
This repository has been archived by the owner on Jan 11, 2023. It is now read-only.

Enable "gosimple" linter plugin #3802

Merged
merged 4 commits into from
Sep 7, 2018
Merged

Enable "gosimple" linter plugin #3802

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
9bdea9f
Simplify code as suggested by gosimple
mboersma Aug 10, 2018
4e9ac75
Prefer helpers.IsTrueBoolPointer for checking variable truth
mboersma Sep 6, 2018
ca83a88
Promote some bool consts to vars in types_test.go
mboersma Sep 6, 2018
98b8d7c
Add "gosimple" plugin and increase linter timeout
mboersma Sep 6, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions pkg/acsengine/defaults_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -618,12 +618,12 @@ func TestSetVMSSDefaults(t *testing.T) {

properties.AgentPoolProfiles[0].Count = 110
setPropertiesDefaults(&mockCS, false, false)
if *properties.AgentPoolProfiles[0].SinglePlacementGroup != false {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lol

if *properties.AgentPoolProfiles[0].SinglePlacementGroup {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need the * syntax here at all?

t.Fatalf("AgentPoolProfile[0].SinglePlacementGroup did not have the expected configuration, got %t, expected %t",
*properties.AgentPoolProfiles[0].SinglePlacementGroup, false)
}

if *properties.AgentPoolProfiles[0].SinglePlacementGroup == false && properties.AgentPoolProfiles[0].StorageProfile != api.ManagedDisks {
if !*properties.AgentPoolProfiles[0].SinglePlacementGroup && properties.AgentPoolProfiles[0].StorageProfile != api.ManagedDisks {
t.Fatalf("AgentPoolProfile[0].StorageProfile did not have the expected configuration, got %s, expected %s",
properties.AgentPoolProfiles[0].StorageProfile, api.ManagedDisks)
}
Expand Down
2 changes: 0 additions & 2 deletions pkg/acsengine/tenantid_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,6 @@ func TestGetTenantID_UnexpectedResponse(t *testing.T) {

mux.HandleFunc("/subscriptions/foobarsubscription", func(w http.ResponseWriter, r *http.Request) {
w.WriteHeader(http.StatusBadRequest)
return
})

_, err := GetTenantID(server.URL, "foobarsubscription")
Expand All @@ -70,7 +69,6 @@ func TestGetTenantID_InvalidHeader(t *testing.T) {
mux.HandleFunc("/subscriptions/foobarsubscription", func(w http.ResponseWriter, r *http.Request) {
w.WriteHeader(http.StatusUnauthorized)
w.Header().Set("fookey", "bazvalue")
return
})

_, err := GetTenantID(server.URL, "foobarsubscription")
Expand Down
2 changes: 1 addition & 1 deletion pkg/acsengine/transform/apimodel_merger.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ type APIModelValue struct {

// MapValues converts an arraw of rwa ApiModel values (like ["masterProfile.count=4","linuxProfile.adminUsername=admin"]) to a map
func MapValues(m map[string]APIModelValue, setFlagValues []string) {
if setFlagValues == nil || len(setFlagValues) == 0 {
if len(setFlagValues) == 0 {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no... really, are you sure you're unset?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pkg/acsengine/transform/apimodel_merger.go:26:5:warning: should omit nil check; len() for nil slices is defined as zero (S1009) (gosimple).

return
}

Expand Down
2 changes: 1 addition & 1 deletion pkg/api/agentPoolOnlyApi/v20180331/validate.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ func (l *LinuxProfile) Validate() error {

// Validate implements APIObject
func (a *AADProfile) Validate(rbacEnabled *bool) error {
if rbacEnabled == nil || *rbacEnabled == false {
if rbacEnabled == nil || !*rbacEnabled {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

see helpers.IsTrueBoolPointer(), I think that's a more expressive way to put the above. E.g.:

if !helper.IsTrueBoolPointer(rbacEnabled) {...

return ErrorRBACNotEnabledForAAD
}

Expand Down
3 changes: 1 addition & 2 deletions pkg/api/converterfromagentpoolonlyapi_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -230,7 +230,7 @@ func TestConvertToV20180331AddonProfile(t *testing.T) {
if _, ok := p[addonName]; !ok {
t.Error("addon is not found")
}
if p[addonName].Enabled != true {
if !p[addonName].Enabled {
t.Error("addon should be enabled")
}
v, ok := p[addonName].Config["opt1"]
Expand All @@ -244,7 +244,6 @@ func TestConvertToV20180331AddonProfile(t *testing.T) {

func TestConvertKubernetesConfigToEnableRBACV20180331AgentPoolOnly(t *testing.T) {
var kc *KubernetesConfig
kc = nil
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lol I'm surprised the compiler accepts this

enableRBAC := convertKubernetesConfigToEnableRBACV20180331AgentPoolOnly(kc)
if enableRBAC == nil {
t.Error("EnableRBAC expected not to be nil")
Expand Down
2 changes: 1 addition & 1 deletion pkg/api/convertertoagentpoolonlyapi.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -384,7 +384,7 @@ func convertV20180331AgentPoolOnlyWindowsProfile(obj *v20180331.WindowsProfile)
}

func convertV20180331AgentPoolOnlyKubernetesConfig(enableRBAC *bool) *KubernetesConfig {
if enableRBAC != nil && *enableRBAC == true {
if enableRBAC != nil && *enableRBAC {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

see my above suggestion

// We set default behavior to be false
return &KubernetesConfig{
EnableRbac: helpers.PointerToBool(true),
Expand Down
21 changes: 10 additions & 11 deletions pkg/api/convertertoagentpoolonlyapi_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -204,7 +204,7 @@ func TestConvertFromV20180331AddonProfile(t *testing.T) {
if _, ok := api[addonName]; !ok {
t.Error("addon is not found")
}
if api[addonName].Enabled != true {
if !api[addonName].Enabled {
t.Error("addon should be enabled")
}
v, ok := api[addonName].Config["opt1"]
Expand All @@ -226,19 +226,18 @@ func TestConvertV20170831AgentPoolOnlyOrchestratorProfile_KubernetesConfig(t *te
t.Error("OrchestratorProfile.KubernetesConfig expected not to be nil")
}

if op.KubernetesConfig.EnableRbac == nil || *op.KubernetesConfig.EnableRbac == true {
if op.KubernetesConfig.EnableRbac == nil || *op.KubernetesConfig.EnableRbac {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ditto

t.Error("OrchestratorProfile.KubernetesConfig.EnableRbac expected to be *false")
}

if op.KubernetesConfig.EnableSecureKubelet == nil || *op.KubernetesConfig.EnableSecureKubelet == true {
if op.KubernetesConfig.EnableSecureKubelet == nil || *op.KubernetesConfig.EnableSecureKubelet {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ditto

t.Error("OrchestratorProfile.KubernetesConfig.EnableSecureKubelet expected to be *false")
}

}

func TestConvertV20180331AgentPoolOnlyKubernetesConfig(t *testing.T) {
var kc *KubernetesConfig
kc = convertV20180331AgentPoolOnlyKubernetesConfig(helpers.PointerToBool(true))
var kc = convertV20180331AgentPoolOnlyKubernetesConfig(helpers.PointerToBool(true))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💥

if kc == nil {
t.Error("kubernetesConfig expected not to be nil")
}
Expand All @@ -247,15 +246,15 @@ func TestConvertV20180331AgentPoolOnlyKubernetesConfig(t *testing.T) {
t.Error("EnableRbac expected not to be nil")
}

if *kc.EnableRbac != true {
if !*kc.EnableRbac {
t.Error("EnableRbac expected to be true")
}

if kc.EnableSecureKubelet == nil {
t.Error("EnableSecureKubelet expected not to be nil")
}

if *kc.EnableSecureKubelet != true {
if !*kc.EnableSecureKubelet {
t.Error("EnableSecureKubelet expected to be true")
}

Expand All @@ -272,15 +271,15 @@ func TestConvertV20180331AgentPoolOnlyKubernetesConfig(t *testing.T) {
t.Error("EnableRbac expected not to be nil")
}

if *kc.EnableRbac != false {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wow

if *kc.EnableRbac {
t.Error("EnableRbac expected to be false")
}

if kc.EnableSecureKubelet == nil {
t.Error("EnableSecureKubelet expected not to be nil")
}

if *kc.EnableSecureKubelet != false {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ditto

if *kc.EnableSecureKubelet {
t.Error("EnableSecureKubelet expected to be false")
}

Expand All @@ -297,15 +296,15 @@ func TestConvertV20180331AgentPoolOnlyKubernetesConfig(t *testing.T) {
t.Error("EnableRbac expected not to be nil")
}

if *kc.EnableRbac != false {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

...

if *kc.EnableRbac {
t.Error("EnableRbac expected to be false")
}

if kc.EnableSecureKubelet == nil {
t.Error("EnableSecureKubelet expected not to be nil")
}

if *kc.EnableSecureKubelet != false {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

make it stop

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It stops here.

if *kc.EnableSecureKubelet {
t.Error("EnableSecureKubelet expected to be false")
}

Expand Down
6 changes: 3 additions & 3 deletions pkg/api/types_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ const exampleUserMSIModel = `{
"orchestratorType": "Kubernetes",
"kubernetesConfig": {
"useManagedIdentity": true,
"userAssignedID": "` + exampleUserMSI + `"
"userAssignedID": "` + exampleUserMSI + `"
}
},
"masterProfile": { "count": 1, "dnsPrefix": "", "vmSize": "Standard_D2_v2" },
Expand Down Expand Up @@ -799,7 +799,7 @@ func TestUserAssignedMSI(t *testing.T) {
}
systemMSI := apiModel.Properties.OrchestratorProfile.KubernetesConfig.UseManagedIdentity
actualUserMSI := apiModel.Properties.OrchestratorProfile.KubernetesConfig.UserAssignedID
if systemMSI != true || actualUserMSI != "" {
if !systemMSI || actualUserMSI != "" {
t.Fatalf("found user msi: %t and usermsi: %s", systemMSI, actualUserMSI)
}

Expand All @@ -814,7 +814,7 @@ func TestUserAssignedMSI(t *testing.T) {
}
systemMSI = apiModel.Properties.OrchestratorProfile.KubernetesConfig.UseManagedIdentity
actualUserMSI = apiModel.Properties.OrchestratorProfile.KubernetesConfig.UserAssignedID
if systemMSI != true && actualUserMSI != exampleUserMSI {
if !systemMSI && actualUserMSI != exampleUserMSI {
t.Fatalf("found user msi: %t and usermsi: %s", systemMSI, actualUserMSI)
}
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/armhelpers/graph.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ func (az *AzureClient) CreateRoleAssignmentSimple(ctx context.Context, resourceG
},
}

re := regexp.MustCompile("(?i)status=(\\d+)")
re := regexp.MustCompile(`(?i)status=(\d+)`)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmm, that extra \, wouldn't that mean that that regex wouldn't be looking for a digit? How did the prior regex even work?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The original regex is a normal, interpreted string literal, so it needed to escape the backslash. Using the backtics syntax makes it a raw string literal, so no escaping is needed. Raw string literals are preferable for regexes for readability and copy-and-pasting.

for {
_, err := az.CreateRoleAssignment(
ctx,
Expand Down
2 changes: 1 addition & 1 deletion pkg/helpers/helpers_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@ EPDesL0rH+3s1CKpgkhYdbJ675GFoGoq+X21QaqsdvoXmmuJF9qq9Tq+JaWloUNq
pemBuffer := bytes.Buffer{}
pem.Encode(&pemBuffer, pemBlock)

if string(pemBuffer.Bytes()) != expectedPrivateKeyString {
if pemBuffer.String() != expectedPrivateKeyString {
t.Fatalf("Private Key did not match expected format/value")
}

Expand Down
3 changes: 1 addition & 2 deletions test/e2e/azure/cli.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -360,8 +360,7 @@ func (a *Account) IsClusterExpired(d time.Duration) bool {

// FetchActivityLog gets all the failures from the activity log for the provided resource group.
func (a *Account) FetchActivityLog(rg string) (string, error) {
var cmd *exec.Cmd
cmd = exec.Command("az", "monitor", "activity-log", "list", "--resource-group", rg, "--status", "Failed")
var cmd = exec.Command("az", "monitor", "activity-log", "list", "--resource-group", rg, "--status", "Failed")
util.PrintCommand(cmd)
out, err := cmd.CombinedOutput()
if err != nil {
Expand Down
11 changes: 2 additions & 9 deletions test/e2e/engine/template.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -236,11 +236,7 @@ func (e *Engine) HasAddon(name string) (bool, api.KubernetesAddon) {

// HasNetworkPolicy will return true if the specified network policy is enabled
func (e *Engine) HasNetworkPolicy(name string) bool {
if strings.Contains(e.ExpandedDefinition.Properties.OrchestratorProfile.KubernetesConfig.NetworkPolicy, name) {
return true
}

return false
return strings.Contains(e.ExpandedDefinition.Properties.OrchestratorProfile.KubernetesConfig.NetworkPolicy, name)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💥 💥

}

// HasAllZonesAgentPools will return true if all of the agent pools have zones
Expand All @@ -251,10 +247,7 @@ func (e *Engine) HasAllZonesAgentPools() bool {
count++
}
}
if count == len(e.ExpandedDefinition.Properties.AgentPoolProfiles) {
return true
}
return false
return count == len(e.ExpandedDefinition.Properties.AgentPoolProfiles)
}

// Write will write the cluster definition to disk
Expand Down
2 changes: 1 addition & 1 deletion test/e2e/kubernetes/deployment/deployment.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -212,7 +212,7 @@ func (d *Deployment) WaitForReplicas(n int, sleep, duration time.Duration) ([]po
select {
case err := <-errCh:
return pods, err
case _ = <-readyCh:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wut

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess you can elide the no-op assignment inside a select statement.

case <-readyCh:
return pods, nil
}
}
Expand Down
2 changes: 1 addition & 1 deletion test/e2e/kubernetes/pod/pod.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -554,7 +554,7 @@ func (p *Pod) ValidateOmsAgentLogs(execCmdString string, sleep, duration time.Du

// CheckWindowsOutboundConnection will keep retrying the check if an error is received until the timeout occurs or it passes. This helps us when DNS may not be available for some time after a pod starts.
func (p *Pod) CheckWindowsOutboundConnection(sleep, duration time.Duration) (bool, error) {
exp, err := regexp.Compile("(StatusCode\\s*:\\s*200)")
exp, err := regexp.Compile(`(StatusCode\s*:\s*200)`)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I'm getting it now that the surrounding quotes were escaping the slashes and then passing those into the regex compiler literally

if err != nil {
log.Printf("Error while trying to create regex for windows outbound check:%s\n", err)
return false, err
Expand Down