Skip to content

Enable gosec/golangci-lint, then fix reported errors #927

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
19 changes: 19 additions & 0 deletions .github/workflows/codegen.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -58,3 +58,22 @@ jobs:
- name: Ensure there is no diff in manifests
run: |
git diff --ignore-matching-lines='.*createdAt:.*' --exit-code -- .

gosec:
name: Ensure that code passes gosec and golint
runs-on: ubuntu-latest
steps:
- name: Checkout code
uses: actions/checkout@v4
- name: Setup Golang
uses: actions/setup-go@v5
with:
go-version-file: 'go.mod'

- name: "Ensure code passes 'go-sec' - run 'make gosec' to identify issues"
run: |
make gosec

- name: "Ensure code passes 'golangci-lint' - run 'make lint' to identify issues"
run: |
make lint
23 changes: 23 additions & 0 deletions .golangci.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
version: "2"
linters:
enable:
- ginkgolinter # https://github.com/nunnatsa/ginkgolinter
exclusions:
generated: lax
presets:
- comments
- common-false-positives
- legacy
- std-error-handling
paths:
- third_party$
- builtin$
- examples$

formatters:
exclusions:
generated: lax
paths:
- third_party$
- builtin$
- examples$
35 changes: 34 additions & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -329,7 +329,6 @@ endif
endif



# A comma-separated list of bundle images (e.g. make catalog-build BUNDLE_IMGS=example.com/operator-bundle:v0.1.0,example.com/operator-bundle:v0.2.0).
# These images MUST exist in a registry and be pull-able.
BUNDLE_IMGS ?= $(BUNDLE_IMG)
Expand All @@ -353,3 +352,37 @@ catalog-build: opm ## Build a catalog image.
.PHONY: catalog-push
catalog-push: ## Push a catalog image.
$(MAKE) docker-push IMG=$(CATALOG_IMG)


.PHONY: gosec
gosec: go_sec
$(GO_SEC) --exclude-dir "hack/upgrade-rollouts-manager" ./...

.PHONY: lint
lint: golangci_lint
$(GOLANGCI_LINT) --version
GOMAXPROCS=2 $(GOLANGCI_LINT) run --fix --verbose --timeout 300s


GO_SEC = $(shell pwd)/bin/gosec
go_sec: ## Download gosec locally if necessary.
$(call go-get-tool,$(GO_SEC),github.com/securego/gosec/v2/cmd/gosec@latest)

GOLANGCI_LINT = $(shell pwd)/bin/golangci-lint
golangci_lint: ## Download golangci-lint locally if necessary.
$(call go-get-tool,$(GOLANGCI_LINT),github.com/golangci/golangci-lint/v2/cmd/golangci-lint@latest)


# go-get-tool will 'go install' any package $2 and install it to $1.
PROJECT_DIR := $(shell dirname $(abspath $(lastword $(MAKEFILE_LIST))))
define go-get-tool
@[ -f $(1) ] || { \
set -e ;\
TMP_DIR=$$(mktemp -d) ;\
cd $$TMP_DIR ;\
go mod init tmp ;\
echo "Downloading $(2)" ;\
GOBIN=$(PROJECT_DIR)/bin go install $(2) ;\
rm -rf $$TMP_DIR ;\
}
endef
8 changes: 3 additions & 5 deletions cmd/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ import (
monitoringv1 "github.com/prometheus-operator/prometheus-operator/pkg/apis/monitoring/v1"
crdv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
"k8s.io/apimachinery/pkg/labels"
"k8s.io/apimachinery/pkg/runtime"

utilruntime "k8s.io/apimachinery/pkg/util/runtime"
clientgoscheme "k8s.io/client-go/kubernetes/scheme"
ctrl "sigs.k8s.io/controller-runtime"
Expand All @@ -56,8 +56,6 @@ import (
"sigs.k8s.io/controller-runtime/pkg/manager"
"sigs.k8s.io/controller-runtime/pkg/webhook"

"github.com/argoproj-labs/argocd-operator/controllers/argocd"

pipelinesv1alpha1 "github.com/redhat-developer/gitops-operator/api/v1alpha1"
"github.com/redhat-developer/gitops-operator/common"
"github.com/redhat-developer/gitops-operator/controllers"
Expand All @@ -69,7 +67,7 @@ import (
)

var (
scheme = runtime.NewScheme()
scheme = k8sruntime.NewScheme()
setupLog = ctrl.Log.WithName("setup")
)

Expand Down Expand Up @@ -257,7 +255,7 @@ func main() {
os.Exit(1)
}

argocd.Register(openshift.ReconcilerHook)
argocdprovisioner.Register(openshift.ReconcilerHook)

setupLog.Info("starting manager")
if err := mgr.Start(ctrl.SetupSignalHandler()); err != nil {
Expand Down
4 changes: 3 additions & 1 deletion controllers/argocd/argocd_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,9 @@ func TestArgoCD(t *testing.T) {
v1.ResourceCPU: resourcev1.MustParse("500m"),
},
}
assert.DeepEqual(t, testArgoCD.Spec.Grafana.Resources, testGrafanaResources)

//lint:ignore SA1019 known to be deprecated
assert.DeepEqual(t, testArgoCD.Spec.Grafana.Resources, testGrafanaResources) //nolint:staticcheck // SA1019: We must test deprecated fields.

testHAResources := &v1.ResourceRequirements{
Requests: v1.ResourceList{
Expand Down
17 changes: 3 additions & 14 deletions controllers/argocd/openshift/clusterconfig_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -97,29 +97,18 @@ func makeTestDeployment() *appsv1.Deployment {
}
}

func makeTestRoleBinding() *rbacv1.RoleBinding {
return &rbacv1.RoleBinding{
ObjectMeta: metav1.ObjectMeta{
Name: testArgoCDName,
Namespace: testNamespace,
},
Subjects: []rbacv1.Subject{},
RoleRef: rbacv1.RoleRef{},
}
}

func newStatefulSetWithSuffix(suffix string, component string, cr *argoapp.ArgoCD) *appsv1.StatefulSet {
return newStatefulSetWithName(fmt.Sprintf("%s-%s", cr.Name, suffix), component, cr)
}

func newStatefulSetWithName(name string, component string, cr *argoapp.ArgoCD) *appsv1.StatefulSet {
ss := newStatefulSet(cr)
ss.ObjectMeta.Name = name
ss.Name = name

lbls := ss.ObjectMeta.Labels
lbls := ss.Labels
lbls[common.ArgoCDKeyName] = name
lbls[common.ArgoCDKeyComponent] = component
ss.ObjectMeta.Labels = lbls
ss.Labels = lbls

return ss
}
Expand Down
18 changes: 10 additions & 8 deletions controllers/argocd/openshift/openshift.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,15 +31,16 @@ func ReconcilerHook(cr *argoapp.ArgoCD, v interface{}, hint string) error {
logv := log.WithValues("ArgoCD Namespace", cr.Namespace, "ArgoCD Name", cr.Name)
switch o := v.(type) {
case *rbacv1.ClusterRole:
if o.ObjectMeta.Name == argocd.GenerateUniqueResourceName("argocd-application-controller", cr) {
if o.Name == argocd.GenerateUniqueResourceName("argocd-application-controller", cr) {
logv.Info("configuring openshift cluster config policy rules")
o.Rules = policyRulesForClusterConfig()
}
case *appsv1.Deployment:
if o.ObjectMeta.Name == cr.ObjectMeta.Name+"-redis" {
switch o.Name {
case cr.Name + "-redis":
logv.Info("configuring openshift redis")
o.Spec.Template.Spec.Containers[0].Args = append(getArgsForRedhatRedis(), o.Spec.Template.Spec.Containers[0].Args...)
} else if o.ObjectMeta.Name == cr.ObjectMeta.Name+"-redis-ha-haproxy" {
case cr.Name + "-redis-ha-haproxy":
logv.Info("configuring openshift redis haproxy")
o.Spec.Template.Spec.Containers[0].Command = append(getCommandForRedhatRedisHaProxy(), o.Spec.Template.Spec.Containers[0].Command...)
version := hint
Expand All @@ -55,13 +56,14 @@ func ReconcilerHook(cr *argoapp.ArgoCD, v interface{}, hint string) error {
}
}
case *appsv1.StatefulSet:
if o.ObjectMeta.Name == cr.ObjectMeta.Name+"-redis-ha-server" {
if o.Name == cr.Name+"-redis-ha-server" {
logv.Info("configuring openshift redis-ha-server stateful set")
for index := range o.Spec.Template.Spec.Containers {
if o.Spec.Template.Spec.Containers[index].Name == "redis" {
switch o.Spec.Template.Spec.Containers[index].Name {
case "redis":
o.Spec.Template.Spec.Containers[index].Args = getArgsForRedhatHaRedisServer()
o.Spec.Template.Spec.Containers[index].Command = []string{}
} else if o.Spec.Template.Spec.Containers[index].Name == "sentinel" {
case "sentinel":
o.Spec.Template.Spec.Containers[index].Args = getArgsForRedhatHaRedisSentinel()
o.Spec.Template.Spec.Containers[index].Command = []string{}
}
Expand All @@ -70,12 +72,12 @@ func ReconcilerHook(cr *argoapp.ArgoCD, v interface{}, hint string) error {
o.Spec.Template.Spec.InitContainers[0].Command = []string{}
}
case *corev1.Secret:
if allowedNamespace(cr.ObjectMeta.Namespace, os.Getenv("ARGOCD_CLUSTER_CONFIG_NAMESPACES")) {
if allowedNamespace(cr.Namespace, os.Getenv("ARGOCD_CLUSTER_CONFIG_NAMESPACES")) {
logv.Info("configuring cluster secret with empty namespaces to allow cluster resources")
delete(o.Data, "namespaces")
}
case *rbacv1.Role:
if o.ObjectMeta.Name == cr.Name+"-"+"argocd-application-controller" {
if o.Name == cr.Name+"-"+"argocd-application-controller" {
logv.Info("configuring policy rule for Application Controller")

// can move this to somewhere common eventually, maybe init()
Expand Down
10 changes: 5 additions & 5 deletions controllers/argocd/openshift/openshift_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -107,13 +107,13 @@ func TestReconcileArgoCD_reconcileRedisDeployment(t *testing.T) {
a := makeTestArgoCD()
testDeployment := makeTestDeployment()

testDeployment.ObjectMeta.Name = a.Name + "-" + "redis"
testDeployment.Name = a.Name + "-" + "redis"
want := append(getArgsForRedhatRedis(), testDeployment.Spec.Template.Spec.Containers[0].Args...)

assert.NoError(t, ReconcilerHook(a, testDeployment, ""))
assert.Equal(t, testDeployment.Spec.Template.Spec.Containers[0].Args, want)

testDeployment.ObjectMeta.Name = a.Name + "-" + "not-redis"
testDeployment.Name = a.Name + "-" + "not-redis"
want = testDeployment.Spec.Template.Spec.Containers[0].Args

assert.NoError(t, ReconcilerHook(a, testDeployment, ""))
Expand All @@ -124,7 +124,7 @@ func TestReconcileArgoCD_reconcileRedisHaProxyDeployment(t *testing.T) {
a := makeTestArgoCD()
testDeployment := makeTestDeployment()

testDeployment.ObjectMeta.Name = a.Name + "-redis-ha-haproxy"
testDeployment.Name = a.Name + "-redis-ha-haproxy"
testDeployment.Spec.Template.Spec.Containers[0].SecurityContext = &corev1.SecurityContext{
Capabilities: &corev1.Capabilities{},
}
Expand All @@ -141,7 +141,7 @@ func TestReconcileArgoCD_reconcileRedisHaProxyDeployment(t *testing.T) {
assert.Equal(t, wantc, *testDeployment.Spec.Template.Spec.Containers[0].SecurityContext.Capabilities)

testDeployment = makeTestDeployment()
testDeployment.ObjectMeta.Name = a.Name + "-redis-ha-haproxy"
testDeployment.Name = a.Name + "-redis-ha-haproxy"
testDeployment.Spec.Template.Spec.Containers[0].SecurityContext = &corev1.SecurityContext{
Capabilities: &corev1.Capabilities{},
}
Expand All @@ -150,7 +150,7 @@ func TestReconcileArgoCD_reconcileRedisHaProxyDeployment(t *testing.T) {
assert.Nil(t, testDeployment.Spec.Template.Spec.Containers[0].SecurityContext.Capabilities)

testDeployment = makeTestDeployment()
testDeployment.ObjectMeta.Name = a.Name + "-" + "not-redis-ha-haproxy"
testDeployment.Name = a.Name + "-" + "not-redis-ha-haproxy"
want = testDeployment.Spec.Template.Spec.Containers[0].Command

assert.NoError(t, ReconcilerHook(a, testDeployment, ""))
Expand Down
3 changes: 2 additions & 1 deletion controllers/argocd_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,8 @@ func TestReconcile_delete_consolelink(t *testing.T) {
t.Run(test.name, func(t *testing.T) {
reconcileArgoCD, fakeClient := newFakeReconcileArgoCD(argoCDRoute, consoleLink)
consoleLink := newConsoleLink("https://test.com", "Cluster Argo CD")
fakeClient.Create(context.TODO(), consoleLink)
err := fakeClient.Create(context.TODO(), consoleLink)
assert.NilError(t, err)

if test.setEnvVarFunc != nil {
test.setEnvVarFunc(t, test.envVar)
Expand Down
14 changes: 7 additions & 7 deletions controllers/argocd_metrics_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -223,8 +223,8 @@ func TestReconcile_add_service_monitors(t *testing.T) {
assert.Assert(t, is.Len(serviceMonitor.OwnerReferences, 1))
assert.Equal(t, serviceMonitor.OwnerReferences[0].Kind, argocdKind)
assert.Equal(t, serviceMonitor.OwnerReferences[0].Name, tc.instanceName)
assert.Equal(t, len(serviceMonitor.ObjectMeta.Labels), 1)
assert.Equal(t, serviceMonitor.ObjectMeta.Labels["release"], "prometheus-operator")
assert.Equal(t, len(serviceMonitor.Labels), 1)
assert.Equal(t, serviceMonitor.Labels["release"], "prometheus-operator")
assert.Equal(t, len(serviceMonitor.Spec.Selector.MatchLabels), 1)
assert.Equal(t, serviceMonitor.Spec.Selector.MatchLabels["app.kubernetes.io/name"], matchLabel)
assert.Equal(t, len(serviceMonitor.Spec.Endpoints), 1)
Expand All @@ -238,8 +238,8 @@ func TestReconcile_add_service_monitors(t *testing.T) {
assert.Assert(t, is.Len(serviceMonitor.OwnerReferences, 1))
assert.Equal(t, serviceMonitor.OwnerReferences[0].Kind, argocdKind)
assert.Equal(t, serviceMonitor.OwnerReferences[0].Name, tc.instanceName)
assert.Equal(t, len(serviceMonitor.ObjectMeta.Labels), 1)
assert.Equal(t, serviceMonitor.ObjectMeta.Labels["release"], "prometheus-operator")
assert.Equal(t, len(serviceMonitor.Labels), 1)
assert.Equal(t, serviceMonitor.Labels["release"], "prometheus-operator")
assert.Equal(t, len(serviceMonitor.Spec.Selector.MatchLabels), 1)
assert.Equal(t, serviceMonitor.Spec.Selector.MatchLabels["app.kubernetes.io/name"], matchLabel)
assert.Equal(t, len(serviceMonitor.Spec.Endpoints), 1)
Expand All @@ -253,8 +253,8 @@ func TestReconcile_add_service_monitors(t *testing.T) {
assert.Assert(t, is.Len(serviceMonitor.OwnerReferences, 1))
assert.Equal(t, serviceMonitor.OwnerReferences[0].Kind, argocdKind)
assert.Equal(t, serviceMonitor.OwnerReferences[0].Name, tc.instanceName)
assert.Equal(t, len(serviceMonitor.ObjectMeta.Labels), 1)
assert.Equal(t, serviceMonitor.ObjectMeta.Labels["release"], "prometheus-operator")
assert.Equal(t, len(serviceMonitor.Labels), 1)
assert.Equal(t, serviceMonitor.Labels["release"], "prometheus-operator")
assert.Equal(t, len(serviceMonitor.Spec.Selector.MatchLabels), 1)
assert.Equal(t, serviceMonitor.Spec.Selector.MatchLabels["app.kubernetes.io/name"], matchLabel)
assert.Equal(t, len(serviceMonitor.Spec.Endpoints), 1)
Expand Down Expand Up @@ -390,7 +390,7 @@ func TestReconciler_add_dashboard(t *testing.T) {
err = r.Client.Get(context.TODO(), types.NamespacedName{Name: name, Namespace: dashboardNamespace}, dashboard)
assert.NilError(t, err)

assert.Assert(t, dashboard.ObjectMeta.Labels["console.openshift.io/dashboard"] == "true")
assert.Assert(t, dashboard.Labels["console.openshift.io/dashboard"] == "true")
assert.Assert(t, dashboard.Data[entry.Name()] == string(content))
}
}
Expand Down
16 changes: 8 additions & 8 deletions controllers/consoleplugin.go
Original file line number Diff line number Diff line change
Expand Up @@ -303,7 +303,7 @@ func (r *ReconcileGitopsService) reconcileDeployment(cr *pipelinesv1alpha1.Gitop
} else {
existingSpecTemplate := &existingPluginDeployment.Spec.Template
newSpecTemplate := newPluginDeployment.Spec.Template
changed := !reflect.DeepEqual(existingPluginDeployment.ObjectMeta.Labels, newPluginDeployment.ObjectMeta.Labels) ||
changed := !reflect.DeepEqual(existingPluginDeployment.Labels, newPluginDeployment.Labels) ||
!reflect.DeepEqual(existingPluginDeployment.Spec.Replicas, newPluginDeployment.Spec.Replicas) ||
!reflect.DeepEqual(existingPluginDeployment.Spec.Selector, newPluginDeployment.Spec.Selector) ||
!reflect.DeepEqual(existingSpecTemplate.Labels, newSpecTemplate.Labels) ||
Expand All @@ -317,7 +317,7 @@ func (r *ReconcileGitopsService) reconcileDeployment(cr *pipelinesv1alpha1.Gitop

if changed {
reqLogger.Info("Reconciling plugin deployment", "Namespace", existingPluginDeployment.Namespace, "Name", existingPluginDeployment.Name)
existingPluginDeployment.ObjectMeta.Labels = newPluginDeployment.ObjectMeta.Labels
existingPluginDeployment.Labels = newPluginDeployment.Labels
existingPluginDeployment.Spec.Replicas = newPluginDeployment.Spec.Replicas
existingPluginDeployment.Spec.Selector = newPluginDeployment.Spec.Selector
existingSpecTemplate.Labels = newSpecTemplate.Labels
Expand Down Expand Up @@ -357,15 +357,15 @@ func (r *ReconcileGitopsService) reconcileService(instance *pipelinesv1alpha1.Gi
return reconcile.Result{}, err
}
} else {
changed := !reflect.DeepEqual(existingServiceRef.ObjectMeta.Annotations, pluginServiceRef.ObjectMeta.Annotations) ||
!reflect.DeepEqual(existingServiceRef.ObjectMeta.Labels, pluginServiceRef.ObjectMeta.Labels) ||
changed := !reflect.DeepEqual(existingServiceRef.Annotations, pluginServiceRef.Annotations) ||
!reflect.DeepEqual(existingServiceRef.Labels, pluginServiceRef.Labels) ||
!reflect.DeepEqual(existingServiceRef.Spec.Selector, pluginServiceRef.Spec.Selector) ||
!reflect.DeepEqual(existingServiceRef.Spec.Ports, pluginServiceRef.Spec.Ports)

if changed {
reqLogger.Info("Reconciling plugin service", "Namespace", existingServiceRef.Namespace, "Name", existingServiceRef.Name)
existingServiceRef.ObjectMeta.Annotations = pluginServiceRef.ObjectMeta.Annotations
existingServiceRef.ObjectMeta.Labels = pluginServiceRef.ObjectMeta.Labels
existingServiceRef.Annotations = pluginServiceRef.Annotations
existingServiceRef.Labels = pluginServiceRef.Labels
existingServiceRef.Spec.Selector = pluginServiceRef.Spec.Selector
existingServiceRef.Spec.Ports = pluginServiceRef.Spec.Ports
return reconcile.Result{}, r.Client.Update(context.TODO(), pluginServiceRef)
Expand Down Expand Up @@ -434,11 +434,11 @@ func (r *ReconcileGitopsService) reconcileConfigMap(instance *pipelinesv1alpha1.
}
} else {
changed := !reflect.DeepEqual(existingPluginConfigMap.Data, newPluginConfigMap.Data) ||
!reflect.DeepEqual(existingPluginConfigMap.ObjectMeta.Labels, newPluginConfigMap.ObjectMeta.Labels)
!reflect.DeepEqual(existingPluginConfigMap.Labels, newPluginConfigMap.Labels)
if changed {
reqLogger.Info("Reconciling plugin configMap", "Namespace", existingPluginConfigMap.Namespace, "Name", existingPluginConfigMap.Name)
existingPluginConfigMap.Data = newPluginConfigMap.Data
existingPluginConfigMap.ObjectMeta.Labels = newPluginConfigMap.ObjectMeta.Labels
existingPluginConfigMap.Labels = newPluginConfigMap.Labels
return reconcile.Result{}, r.Client.Update(context.TODO(), newPluginConfigMap)
}
}
Expand Down
Loading
Loading