Skip to content
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

[release-2.7] Update to Go v1.20 #223

Merged
merged 1 commit into from
Aug 2, 2023
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
2 changes: 1 addition & 1 deletion .ci-operator.yaml
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
build_root_image:
name: builder
namespace: stolostron
tag: go1.19-linux
tag: go1.20-linux
2 changes: 1 addition & 1 deletion Dockerfile
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
# Copyright Contributors to the Open Cluster Management project

# Stage 1: Use image builder to build the target binaries
FROM registry.ci.openshift.org/stolostron/builder:go1.19-linux AS builder
FROM registry.ci.openshift.org/stolostron/builder:go1.20-linux AS builder

ENV COMPONENT=cert-policy-controller
ENV REPO_PATH=/go/src/github.com/stolostron/${COMPONENT}
Expand Down
4 changes: 2 additions & 2 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ create-ns:
# Lint code
.PHONY: lint-dependencies
lint-dependencies:
$(call go-get-tool,github.com/golangci/golangci-lint/cmd/golangci-lint@v1.46.2)
$(call go-get-tool,github.com/golangci/golangci-lint/cmd/golangci-lint@v1.52.2)

.PHONY: lint
lint: lint-dependencies lint-all
Expand Down Expand Up @@ -206,7 +206,7 @@ kubebuilder-dependencies: $(LOCAL_BIN)

.PHONY: gosec
gosec:
$(call go-get-tool,github.com/securego/gosec/v2/cmd/gosec@v2.9.6)
$(call go-get-tool,github.com/securego/gosec/v2/cmd/gosec@v2.15.0)

.PHONY: gosec-scan
gosec-scan: gosec
Expand Down
8 changes: 4 additions & 4 deletions api/v1/certificatepolicy_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -89,21 +89,21 @@ type CertificatePolicySpec struct {
MinDuration *metav1.Duration `json:"minimumDuration,omitempty"`
// Minimum CA duration before a signing certificate expires that it is considered non-compliant.
// Golang's time units only.
MinCADuration *metav1.Duration `json:"minimumCADuration,omitempty"` // nolint:tagliatelle
MinCADuration *metav1.Duration `json:"minimumCADuration,omitempty"` //nolint:tagliatelle
// Maximum duration for a certificate, longer duration is considered non-compliant.
// Golang's time units only.
MaxDuration *metav1.Duration `json:"maximumDuration,omitempty"`
// Maximum CA duration for a signing certificate, longer duration is considered non-compliant.
// Golang's time units only.
MaxCADuration *metav1.Duration `json:"maximumCADuration,omitempty"` // nolint:tagliatelle
MaxCADuration *metav1.Duration `json:"maximumCADuration,omitempty"` //nolint:tagliatelle
// A pattern that must match any defined SAN entries in the certificate for the certificate to be compliant.
// Golang's regexp syntax only.
// +kubebuilder:validation:MinLength=1
AllowedSANPattern string `json:"allowedSANPattern,omitempty"` // nolint:tagliatelle
AllowedSANPattern string `json:"allowedSANPattern,omitempty"` //nolint:tagliatelle
// A pattern that must not match any defined SAN entries in the certificate for the certificate to be compliant.
// Golang's regexp syntax only.
// +kubebuilder:validation:MinLength=1
DisallowedSANPattern string `json:"disallowedSANPattern,omitempty"` // nolint:tagliatelle
DisallowedSANPattern string `json:"disallowedSANPattern,omitempty"` //nolint:tagliatelle
}

// CertificatePolicyStatus defines the observed state of CertificatePolicy
Expand Down
35 changes: 20 additions & 15 deletions controllers/certificatepolicy_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ func (r *CertificatePolicyReconciler) Reconcile(ctx context.Context, request ctr

instance.Status.CompliancyDetails = make(map[string]policyv1.CompliancyDetails)

r.handleAddingPolicy(instance)
r.handleAddingPolicy(ctx, instance)
}

reqLogger.V(1).Info("Successful processing", "instance.Name", instance.Name, "instance.Namespace",
Expand Down Expand Up @@ -173,7 +173,9 @@ func ensureDefaultLabel(instance *policyv1.CertificatePolicy) bool {
}

// PeriodicallyExecCertificatePolicies always check status - let this be the only function in the controller.
func (r *CertificatePolicyReconciler) PeriodicallyExecCertificatePolicies(freq uint, loopflag bool) {
func (r *CertificatePolicyReconciler) PeriodicallyExecCertificatePolicies(
ctx context.Context, freq uint, loopflag bool,
) {
log.V(3).Info("Entered PeriodicallyExecCertificatePolicies")
var plcToUpdateMap map[string]*policyv1.CertificatePolicy

Expand All @@ -184,11 +186,11 @@ func (r *CertificatePolicyReconciler) PeriodicallyExecCertificatePolicies(freq u

plcToUpdateMap = make(map[string]*policyv1.CertificatePolicy)

stateChange := r.ProcessPolicies(plcToUpdateMap)
stateChange := r.ProcessPolicies(ctx, plcToUpdateMap)

if stateChange {
// update status of all policies that changed:
faultyPlc, err := r.updatePolicyStatus(plcToUpdateMap)
faultyPlc, err := r.updatePolicyStatus(ctx, plcToUpdateMap)
if err != nil {
log.Error(err, "Unable to update policy status", "Name", faultyPlc.Name, "Namespace",
faultyPlc.Namespace)
Expand All @@ -211,7 +213,9 @@ func (r *CertificatePolicyReconciler) PeriodicallyExecCertificatePolicies(freq u
}

// ProcessPolicies reads each policy and looks for violations returning true if a change is found.
func (r *CertificatePolicyReconciler) ProcessPolicies(plcToUpdateMap map[string]*policyv1.CertificatePolicy) bool {
func (r *CertificatePolicyReconciler) ProcessPolicies(
ctx context.Context, plcToUpdateMap map[string]*policyv1.CertificatePolicy,
) bool {
stateChange := false

plcMap := make(map[string]*policyv1.CertificatePolicy)
Expand All @@ -222,7 +226,7 @@ func (r *CertificatePolicyReconciler) ProcessPolicies(plcToUpdateMap map[string]
// update available policies if there are changed namespaces
for _, plc := range plcMap {
// Retrieve the namespaces based on filters in NamespaceSelector
selectedNamespaces := r.retrieveNamespaces(plc.Spec.NamespaceSelector)
selectedNamespaces := r.retrieveNamespaces(ctx, plc.Spec.NamespaceSelector)

// add availablePolicy if not present
for _, ns := range selectedNamespaces {
Expand Down Expand Up @@ -261,7 +265,7 @@ func (r *CertificatePolicyReconciler) ProcessPolicies(plcToUpdateMap map[string]

log.V(2).Info("Checking certificates", "namespace", namespace, "policy.Name", policy.Name)

update, nonCompliant, list := r.checkSecrets(policy, namespace)
update, nonCompliant, list := r.checkSecrets(ctx, policy, namespace)

if strings.EqualFold(string(policy.Spec.RemediationAction), string(policyv1.Enforce)) {
log.V(1).Info("Enforce is set, but not implemented on this controller")
Expand Down Expand Up @@ -337,7 +341,7 @@ func toLabelSet(v map[string]policyv1.NonEmptyString) labels.Set {
// Checks each namespace for certificates that are going to expire within 3 months
// Returns whether a state change is happening, the number of uncompliant certificates
// and a list of the uncompliant certificates.
func (r *CertificatePolicyReconciler) checkSecrets(policy *policyv1.CertificatePolicy,
func (r *CertificatePolicyReconciler) checkSecrets(ctx context.Context, policy *policyv1.CertificatePolicy,
namespace string,
) (bool, uint, map[string]policyv1.Cert) {
slog := log.WithValues("policy.Namespace", policy.Namespace, "policy.Name", policy.Name)
Expand All @@ -352,7 +356,7 @@ func (r *CertificatePolicyReconciler) checkSecrets(policy *policyv1.CertificateP
// GOAL: Want the label selector to find secrets with certificates only!! -> is-certificate
// Loops through all the secrets within the CertificatePolicy's specified namespace
labelSelector := toLabelSet(policy.Spec.LabelSelector)
secretList, _ := r.TargetK8sClient.CoreV1().Secrets(namespace).List(context.TODO(),
secretList, _ := r.TargetK8sClient.CoreV1().Secrets(namespace).List(ctx,
metav1.ListOptions{LabelSelector: labelSelector.String()})

for _, secretItem := range secretList.Items {
Expand Down Expand Up @@ -381,14 +385,14 @@ func (r *CertificatePolicyReconciler) checkSecrets(policy *policyv1.CertificateP
return update, uint(len(nonCompliantCertificates)), nonCompliantCertificates
}

func (r *CertificatePolicyReconciler) retrieveNamespaces(selector policyv1.Target) []string {
func (r *CertificatePolicyReconciler) retrieveNamespaces(ctx context.Context, selector policyv1.Target) []string {
var selectedNamespaces []string
// If MatchLabels/MatchExpressions/Include were not provided, return no namespaces
if selector.MatchLabels == nil && selector.MatchExpressions == nil && len(selector.Include) == 0 {
log.Info("NamespaceSelector is empty. Skipping namespace retrieval.")
} else {
var err error
selectedNamespaces, err = common.GetSelectedNamespaces(r.TargetK8sClient, selector)
selectedNamespaces, err = common.GetSelectedNamespaces(ctx, r.TargetK8sClient, selector)
if err != nil {
log.Error(
err, "Error filtering namespaces with provided NamespaceSelector",
Expand Down Expand Up @@ -745,7 +749,8 @@ func checkComplianceChangeBasedOnDetails(plc *policyv1.CertificatePolicy) (compl
return reflect.DeepEqual(previous, plc.Status.ComplianceState)
}

func (r *CertificatePolicyReconciler) updatePolicyStatus(policies map[string]*policyv1.CertificatePolicy,
func (r *CertificatePolicyReconciler) updatePolicyStatus(
ctx context.Context, policies map[string]*policyv1.CertificatePolicy,
) (*policyv1.CertificatePolicy, error) {
log.V(3).Info("Entered updatePolicyStatus")

Expand All @@ -769,7 +774,7 @@ func (r *CertificatePolicyReconciler) updatePolicyStatus(policies map[string]*po
}
}

err := r.Status().Update(context.TODO(), instance)
err := r.Status().Update(ctx, instance)
if err != nil {
return instance, err
}
Expand Down Expand Up @@ -800,7 +805,7 @@ func handleRemovingPolicy(name string) {
}
}

func (r *CertificatePolicyReconciler) handleAddingPolicy(plc *policyv1.CertificatePolicy) {
func (r *CertificatePolicyReconciler) handleAddingPolicy(ctx context.Context, plc *policyv1.CertificatePolicy) {
log.V(3).Info("Entered handleAddingPolicy")

// clean up that policy from the availablePolicies list, in case the modification is in the
Expand All @@ -816,7 +821,7 @@ func (r *CertificatePolicyReconciler) handleAddingPolicy(plc *policyv1.Certifica
addFlag := false

// Retrieve the namespaces based on filters in NamespaceSelector
selectedNamespaces := r.retrieveNamespaces(plc.Spec.NamespaceSelector)
selectedNamespaces := r.retrieveNamespaces(ctx, plc.Spec.NamespaceSelector)

for _, ns := range selectedNamespaces {
key := fmt.Sprintf("%s/%s", ns, plc.Name)
Expand Down
26 changes: 13 additions & 13 deletions controllers/certificatepolicy_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -174,8 +174,8 @@ func TestPeriodicallyExecCertificatePolicies(t *testing.T) {
certPolicy.Name = fmt.Sprintf("%s-%d", certPolicy.Name, i)
certPolicy.Spec.NamespaceSelector.Include = []policiesv1.NonEmptyString{test.namespaceSelector}

r.handleAddingPolicy(certPolicy)
r.PeriodicallyExecCertificatePolicies(1, false)
r.handleAddingPolicy(context.TODO(), certPolicy)
r.PeriodicallyExecCertificatePolicies(context.TODO(), 1, false)

policy, found := availablePolicies.GetObject(test.cacheNamespace + "/" + certPolicy.Name)
assert.True(t, found)
Expand All @@ -192,7 +192,7 @@ func TestPeriodicallyExecCertificatePolicies(t *testing.T) {
}
}

func TestCheckComplianceBasedOnDetails(t *testing.T) {
func TestCheckComplianceBasedOnDetails(_ *testing.T) {
certPolicy := policiesv1.CertificatePolicy{
ObjectMeta: metav1.ObjectMeta{
Name: "foo",
Expand Down Expand Up @@ -298,14 +298,14 @@ func TestHandleAddingPolicy(t *testing.T) {
}
certPolicy.Spec.NamespaceSelector.Include = []policiesv1.NonEmptyString{"default"}

r.handleAddingPolicy(certPolicy)
r.handleAddingPolicy(context.TODO(), certPolicy)
policy, found := availablePolicies.GetObject(certPolicy.Namespace + "/" + certPolicy.Name)
assert.True(t, found)
assert.NotNil(t, policy)
handleRemovingPolicy(certPolicy.Name)
}

func TestPrintMap(t *testing.T) {
func TestPrintMap(_ *testing.T) {
certPolicy := policiesv1.CertificatePolicy{
ObjectMeta: metav1.ObjectMeta{
Name: "foo",
Expand Down Expand Up @@ -491,10 +491,10 @@ func TestProcessPolicies(t *testing.T) {
},
}
r := &CertificatePolicyReconciler{Client: nil, Scheme: nil, Recorder: nil, TargetK8sClient: nil}
r.handleAddingPolicy(instance)
r.handleAddingPolicy(context.TODO(), instance)

plcToUpdateMap := make(map[string]*policiesv1.CertificatePolicy)
value := r.ProcessPolicies(plcToUpdateMap)
value := r.ProcessPolicies(context.TODO(), plcToUpdateMap)
assert.True(t, value)

_, found := availablePolicies.GetObject("/" + instance.Name)
Expand Down Expand Up @@ -587,7 +587,7 @@ uFPO5+jBaPT3/G0z1dDrZZDOxhTSkFuyLTXnaEhIbZQW0Mniq1m5nswOAgfompmA
target := []policiesv1.NonEmptyString{"default"}
instance.Spec.NamespaceSelector.Include = target

r.handleAddingPolicy(instance)
r.handleAddingPolicy(context.TODO(), instance)

policy, found := availablePolicies.GetObject(instance.Namespace + "/" + instance.Name)
assert.True(t, found)
Expand All @@ -605,7 +605,7 @@ uFPO5+jBaPT3/G0z1dDrZZDOxhTSkFuyLTXnaEhIbZQW0Mniq1m5nswOAgfompmA
assert.Nil(t, err)
assert.NotNil(t, cert)

update, nonCompliant, list := r.checkSecrets(instance, "default")
update, nonCompliant, list := r.checkSecrets(context.TODO(), instance, "default")

assert.Nil(t, err)
assert.Equal(t, uint(1), nonCompliant)
Expand Down Expand Up @@ -664,7 +664,7 @@ xUSmOkQ0VchHrQY4a3z4yzgWIdDe34DhonLA1njXcd66kzY5cD1EykmLcIPFLqCx

target = []policiesv1.NonEmptyString{"default"}
instance.Spec.NamespaceSelector.Include = target
r.handleAddingPolicy(instance)
r.handleAddingPolicy(context.TODO(), instance)

policy, found = availablePolicies.GetObject(instance.Namespace + "/" + instance.Name)
assert.True(t, found)
Expand All @@ -678,7 +678,7 @@ xUSmOkQ0VchHrQY4a3z4yzgWIdDe34DhonLA1njXcd66kzY5cD1EykmLcIPFLqCx
)
assert.Equal(t, 2, len(secretList.Items))

update, nonCompliant, list = r.checkSecrets(instance, "default")
update, nonCompliant, list = r.checkSecrets(context.TODO(), instance, "default")

assert.Nil(t, err)
assert.Equal(t, uint(2), nonCompliant)
Expand Down Expand Up @@ -802,15 +802,15 @@ uFPO5+jBaPT3/G0z1dDrZZDOxhTSkFuyLTXnaEhIbZQW0Mniq1m5nswOAgfompmA
target := []policiesv1.NonEmptyString{"def*"}
instance.Spec.NamespaceSelector.Include = target

r.handleAddingPolicy(instance)
r.handleAddingPolicy(context.TODO(), instance)

policy, found := availablePolicies.GetObject(instance.Namespace + "/" + instance.Name)
assert.True(t, found)
assert.NotNil(t, policy)

plcToUpdateMap := make(map[string]*policiesv1.CertificatePolicy)

stateChange := r.ProcessPolicies(plcToUpdateMap)
stateChange := r.ProcessPolicies(context.TODO(), plcToUpdateMap)
assert.True(t, stateChange)

message := convertPolicyStatusToString(instance, DefaultDuration)
Expand Down
2 changes: 1 addition & 1 deletion controllers/certificatepolicy_utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ func convertPolicyStatusToString(plc *policyv1.CertificatePolicy, defaultDuratio
}

// Message format:
// NonCompliant; x certificates expire in less than 300h: namespace:secretname, namespace:secretname, ...
// NonCompliant; x certificates expire in less than 300h: namespace:secretname1, namespace:secretname2, ...
expireCount := 0
expireCACount := 0
durationCount := 0
Expand Down
3 changes: 1 addition & 2 deletions controllers/suite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ limitations under the License.

package controllers

// nolint:gci
//nolint:gci
import (
"path/filepath"
"testing"
Expand All @@ -31,7 +31,6 @@ import (
"sigs.k8s.io/controller-runtime/pkg/log/zap"

policyv1 "open-cluster-management.io/cert-policy-controller/api/v1"
//+kubebuilder:scaffold:imports
)

// These tests use Ginkgo (BDD-style Go testing framework). Refer to
Expand Down
3 changes: 1 addition & 2 deletions go.mod
Original file line number Diff line number Diff line change
@@ -1,9 +1,8 @@
module open-cluster-management.io/cert-policy-controller

go 1.19
go 1.20

require (
github.com/ghodss/yaml v1.0.1-0.20190212211648-25d852aebe32
github.com/go-logr/zapr v1.2.3
github.com/onsi/ginkgo/v2 v2.1.4
github.com/onsi/gomega v1.19.0
Expand Down
2 changes: 0 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -163,8 +163,6 @@ github.com/fsnotify/fsnotify v1.5.1/go.mod h1:T3375wBYaZdLLcVNkcVbzGHY7f1l/uK5T5
github.com/getkin/kin-openapi v0.76.0/go.mod h1:660oXbgy5JFMKreazJaQTw7o+X00qeSyhcnluiMv+Xg=
github.com/getsentry/raven-go v0.2.0/go.mod h1:KungGk8q33+aIAZUIVWZDr2OfAEBsO49PX4NzFV5kcQ=
github.com/ghodss/yaml v1.0.0/go.mod h1:4dBDuWmgqj2HViK6kFavaiC9ZROes6MMH2rRYeMEF04=
github.com/ghodss/yaml v1.0.1-0.20190212211648-25d852aebe32 h1:Mn26/9ZMNWSw9C9ERFA1PUxfmGpolnw2v0bKOREu5ew=
github.com/ghodss/yaml v1.0.1-0.20190212211648-25d852aebe32/go.mod h1:GIjDIg/heH5DOkXY3YJ/wNhfHsQHoXGjl8G8amsYQ1I=
github.com/go-gl/glfw v0.0.0-20190409004039-e6da0acd62b1/go.mod h1:vR7hzQXu2zJy9AVAgeJqvqgH9Q5CA+iKCZ2gyEVpxRU=
github.com/go-gl/glfw/v3.3/glfw v0.0.0-20191125211704-12ad95a8df72/go.mod h1:tQ2UAYgL5IevRw8kRxooKSPJfGvJ9fJQFa0TUsXzTg8=
github.com/go-gl/glfw/v3.3/glfw v0.0.0-20200222043503-6f7a984d4dc4/go.mod h1:tQ2UAYgL5IevRw8kRxooKSPJfGvJ9fJQFa0TUsXzTg8=
Expand Down
6 changes: 3 additions & 3 deletions main.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@

package main

// nolint:gci
//nolint:gci
import (
"context"
"errors"
Expand Down Expand Up @@ -56,7 +56,7 @@ func printVersion() {
"GOOS", runtime.GOOS, "GOARCH", runtime.GOARCH)
}

// nolint:wsl
//nolint:wsl
func init() {
utilruntime.Must(clientgoscheme.AddToScheme(scheme))
utilruntime.Must(extpolicyv1.AddToScheme(scheme))
Expand Down Expand Up @@ -292,7 +292,7 @@ func main() {
_ = r.Initialize(namespace, eventOnParent, time.Duration(0)) /* #nosec G104 */
// PeriodicallyExecCertificatePolicies is the go-routine that periodically checks the policies and
// does the needed work to make sure the desired state is achieved
go r.PeriodicallyExecCertificatePolicies(frequency, true)
go r.PeriodicallyExecCertificatePolicies(context.TODO(), frequency, true)

if enableLease {
startLeaseController(generatedClient, hubConfigPath, clusterName)
Expand Down
Loading