Skip to content

Commit

Permalink
address gosec lint issues (#5183)
Browse files Browse the repository at this point in the history
* address gosec G101 - hardcoded credentials issues
* address G601: Implicit memory aliasing in for loop. (gosec)
  • Loading branch information
pdabelf5 authored Feb 29, 2024
1 parent 8ed2ae5 commit 1fefb7d
Show file tree
Hide file tree
Showing 10 changed files with 30 additions and 7 deletions.
5 changes: 5 additions & 0 deletions .golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -61,5 +61,10 @@ issues:
max-issues-per-linter: 0
max-same-issues: 0
exclude-use-default: false
exclude-rules:
- path: _test\.go
text: "Potential hardcoded credentials"
linters:
- gosec
run:
timeout: 5m
3 changes: 2 additions & 1 deletion internal/configs/configurator.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ const (
)

// DefaultServerSecretPath is the full path to the Secret with a TLS cert and a key for the default server. #nosec G101
const DefaultServerSecretPath = "/etc/nginx/secrets/default"
const DefaultServerSecretPath = "/etc/nginx/secrets/default" //nolint:gosec // G101: Potential hardcoded credentials - false positive

// DefaultSecretPath is the full default path to where secrets are stored and accessed.
const DefaultSecretPath = "/etc/nginx/secrets" // #nosec G101
Expand Down Expand Up @@ -1166,6 +1166,7 @@ func (cnf *Configurator) updatePlusEndpoints(ingEx *IngressEx) error {
}

for _, path := range rule.HTTP.Paths {
path := path // address gosec G601
endps, exists := ingEx.Endpoints[path.Backend.Service.Name+GetBackendPortAsString(path.Backend.Service.Port)]
if exists {
if _, isExternalName := ingEx.ExternalNameSvcs[path.Backend.Service.Name]; isExternalName {
Expand Down
2 changes: 2 additions & 0 deletions internal/k8s/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -1171,6 +1171,7 @@ func (lbc *LoadBalancerController) cleanupUnwatchedNamespacedResources(nsi *name
glog.Warningf("unable to list Ingress resources for recently unwatched namespace %s", nsi.namespace)
} else {
for _, ing := range il.Items {
ing := ing // address gosec G601
key := getResourceKey(&ing.ObjectMeta)
delIngressList = append(delIngressList, key)
lbc.configuration.DeleteIngress(key)
Expand Down Expand Up @@ -2908,6 +2909,7 @@ func (lbc *LoadBalancerController) createIngressEx(ing *networking.Ingress, vali
}

for _, path := range rule.HTTP.Paths {
path := path // address gosec G601
podEndps := []podEndpoint{}
if validMinionPaths != nil && !validMinionPaths[path.Path] {
glog.V(3).Infof("Skipping path %s for minion Ingress %s", path.Path, ing.Name)
Expand Down
7 changes: 7 additions & 0 deletions internal/k8s/controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -579,6 +579,7 @@ func TestGetEndpointsFromEndpointSlices_DuplicateEndpointsInOneEndpointSlice(t *
}

for _, test := range tests {
test := test // address gosec G601
t.Run(test.desc, func(t *testing.T) {
gotEndpoints, err := lbc.getEndpointsForPortFromEndpointSlices(test.svcEndpointSlices, backendServicePort, &test.svc)
if err != nil {
Expand Down Expand Up @@ -670,6 +671,7 @@ func TestGetEndpointsFromEndpointSlices_TwoDifferentEndpointsInOnEndpointSlice(t
}

for _, test := range tests {
test := test // address gosec G601
t.Run(test.desc, func(t *testing.T) {
gotEndpoints, err := lbc.getEndpointsForPortFromEndpointSlices(test.svcEndpointSlices, backendServicePort, &test.svc)
if err != nil {
Expand Down Expand Up @@ -790,6 +792,7 @@ func TestGetEndpointsFromEndpointSlices_DuplicateEndpointsAcrossTwoEndpointSlice
}

for _, test := range tests {
test := test // address gosec G601
t.Run(test.desc, func(t *testing.T) {
gotEndpoints, err := lbc.getEndpointsForPortFromEndpointSlices(test.svcEndpointSlices, backendServicePort, &test.svc)
if err != nil {
Expand Down Expand Up @@ -879,6 +882,7 @@ func TestGetEndpointsFromEndpointSlices_TwoDifferentEndpointsInOnEndpointSliceOn
}

for _, test := range tests {
test := test // address gosec G601
t.Run(test.desc, func(t *testing.T) {
gotEndpoints, err := lbc.getEndpointsForPortFromEndpointSlices(test.svcEndpointSlices, backendServicePort, &test.svc)
if err != nil {
Expand Down Expand Up @@ -978,6 +982,7 @@ func TestGetEndpointsFromEndpointSlices_TwoDifferentEndpointsAcrossTwoEndpointSl
}

for _, test := range tests {
test := test // address gosec G601
t.Run(test.desc, func(t *testing.T) {
gotEndpoints, err := lbc.getEndpointsForPortFromEndpointSlices(test.svcEndpointSlices, backendServicePort, &test.svc)
if err != nil {
Expand Down Expand Up @@ -1053,6 +1058,7 @@ func TestGetEndpointsFromEndpointSlices_ErrorsOnInvalidTargetPort(t *testing.T)
}

for _, test := range tests {
test := test // address gosec G601
t.Run(test.desc, func(t *testing.T) {
_, err := lbc.getEndpointsForPortFromEndpointSlices(test.svcEndpointSlices, backendServicePort, &test.svc)
if err == nil {
Expand Down Expand Up @@ -1103,6 +1109,7 @@ func TestGetEndpointsFromEndpointSlices_ErrorsOnNoEndpointSlicesFound(t *testing
}

for _, test := range tests {
test := test // address gosec G601
t.Run(test.desc, func(t *testing.T) {
_, err := lbc.getEndpointsForPortFromEndpointSlices(test.svcEndpointSlices, backendServicePort, &test.svc)
if err == nil {
Expand Down
6 changes: 3 additions & 3 deletions internal/k8s/secrets/validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,13 +23,13 @@ const ClientSecretKey = "client-secret"
const HtpasswdFileKey = "htpasswd"

// SecretTypeCA contains a certificate authority for TLS certificate verification. #nosec G101
const SecretTypeCA api_v1.SecretType = "nginx.org/ca"
const SecretTypeCA api_v1.SecretType = "nginx.org/ca" //nolint:gosec // G101: Potential hardcoded credentials - false positive

// SecretTypeJWK contains a JWK (JSON Web Key) for validating JWTs (JSON Web Tokens). #nosec G101
const SecretTypeJWK api_v1.SecretType = "nginx.org/jwk"
const SecretTypeJWK api_v1.SecretType = "nginx.org/jwk" //nolint:gosec // G101: Potential hardcoded credentials - false positive

// SecretTypeOIDC contains an OIDC client secret for use in oauth flows. #nosec G101
const SecretTypeOIDC api_v1.SecretType = "nginx.org/oidc"
const SecretTypeOIDC api_v1.SecretType = "nginx.org/oidc" //nolint:gosec // G101: Potential hardcoded credentials - false positive

// SecretTypeHtpasswd contains an htpasswd file for use in HTTP Basic authorization.. #nosec G101
const SecretTypeHtpasswd api_v1.SecretType = "nginx.org/htpasswd" // #nosec G101
Expand Down
3 changes: 3 additions & 0 deletions internal/k8s/status_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -500,6 +500,7 @@ func TestHasVsStatusChanged(t *testing.T) {
}

for _, test := range tests {
test := test // address gosec G601
changed := hasVsStatusChanged(&test.vs, state, reason, msg)

if changed != test.expected {
Expand Down Expand Up @@ -577,6 +578,7 @@ func TestHasVsrStatusChanged(t *testing.T) {
}

for _, test := range tests {
test := test // address gosec G601
changed := hasVsrStatusChanged(&test.vsr, state, reason, msg, referencedBy)

if changed != test.expected {
Expand Down Expand Up @@ -728,6 +730,7 @@ func TestHasPolicyStatusChanged(t *testing.T) {
}

for _, test := range tests {
test := test // address gosec G601
changed := hasPolicyStatusChanged(&test.pol, state, reason, msg)

if changed != test.expected {
Expand Down
1 change: 1 addition & 0 deletions internal/k8s/validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -829,6 +829,7 @@ func validateIngressSpec(spec *networking.IngressSpec, fieldPath *field.Path) fi
}

for _, path := range r.HTTP.Paths {
path := path // address gosec G601
idxPath := idxRule.Child("http").Child("path").Index(i)

allErrs = append(allErrs, validatePath(path.Path, path.PathType, idxPath.Child("path"))...)
Expand Down
1 change: 1 addition & 0 deletions pkg/apis/configuration/validation/virtualserver.go
Original file line number Diff line number Diff line change
Expand Up @@ -572,6 +572,7 @@ func (vsv *VirtualServerValidator) validateUpstreams(upstreams []v1.Upstream, fi
upstreamNames = sets.Set[string]{}

for i, u := range upstreams {
u := u // address gosec G601
idxPath := fieldPath.Index(i)

upstreamErrors := validateUpstreamName(u.Name, idxPath.Child("name"))
Expand Down
7 changes: 4 additions & 3 deletions pkg/apis/configuration/validation/virtualserver_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4359,7 +4359,7 @@ func TestValidateErrorPageReturn(t *testing.T) {
vsv := &VirtualServerValidator{isPlus: false}

for _, epr := range tests {
// FIXME #nosec G601
epr := epr // address gosec G601
allErrs := vsv.validateErrorPageReturn(&epr, field.NewPath("return"))
if len(allErrs) != 0 {
t.Errorf("validateErrorPageReturn(%v) returned errors for valid input: %v", epr, allErrs)
Expand Down Expand Up @@ -4424,6 +4424,7 @@ func TestValidateErrorPageReturnFails(t *testing.T) {
vsv := &VirtualServerValidator{isPlus: false}

for _, test := range tests {
test := test // address gosec G601
allErrs := vsv.validateErrorPageReturn(&test.epr, field.NewPath("return"))
if len(allErrs) == 0 {
t.Errorf("validateErrorPageReturn(%v) returned no errors for invalid input for the case of %v", test.epr, test.msg)
Expand Down Expand Up @@ -4451,7 +4452,7 @@ func TestValidateErrorPageRedirect(t *testing.T) {
vsv := &VirtualServerValidator{isPlus: false}

for _, epr := range tests {
// FIXME #nosec G601
epr := epr // address gosec G601
allErrs := vsv.validateErrorPageRedirect(&epr, field.NewPath("redirect"))
if len(allErrs) != 0 {
t.Errorf("validateErrorPageRedirect(%v) returned errors for valid input: %v", epr, allErrs)
Expand Down Expand Up @@ -4497,7 +4498,7 @@ func TestValidateErrorPageRedirectFails(t *testing.T) {
vsv := &VirtualServerValidator{isPlus: false}

for _, epr := range tests {
// FIXME #nosec G601
epr := epr // address gosec G601
allErrs := vsv.validateErrorPageRedirect(&epr, field.NewPath("redirect"))
if len(allErrs) == 0 {
t.Errorf("validateErrorPageRedirect(%v) returned no errors for invalid input", epr)
Expand Down
2 changes: 2 additions & 0 deletions pkg/apis/externaldns/validation/externaldns_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,7 @@ func TestValidateDNSEndpoint(t *testing.T) {
}

for _, tc := range tt {
tc := tc // address gosec G601
t.Run(tc.name, func(t *testing.T) {
if err := validation.ValidateDNSEndpoint(&tc.endpoint); err != nil {
t.Errorf("want no error on %v, got %v", tc.endpoint, err)
Expand Down Expand Up @@ -282,6 +283,7 @@ func TestValidateDNSEndpoint_ReturnsErrorOn(t *testing.T) {
}

for _, tc := range tt {
tc := tc // address gosec G601
t.Run(tc.name, func(t *testing.T) {
err := validation.ValidateDNSEndpoint(&tc.endpoint)
if !errors.Is(err, tc.want) {
Expand Down

0 comments on commit 1fefb7d

Please sign in to comment.