Skip to content

Commit

Permalink
feat(lint): configures importas for consistent import aliases
Browse files Browse the repository at this point in the history
This linter makes sure that we use consitent aliases for imports, which are especially handy when it comes to versioned api packages. With simple rules we can ensure that e.g. "k8s.io/api/core/v1" is consistently aliased as corev1. In case of "k8s.io/apimachinery/pkg/api/errors" it will expect k8serr alias instead of apierrs, as former is more descriptive.
  • Loading branch information
bartoszmajsak committed Jun 27, 2024
1 parent fe73ce1 commit 076faae
Show file tree
Hide file tree
Showing 7 changed files with 57 additions and 38 deletions.
39 changes: 29 additions & 10 deletions .golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -43,18 +43,37 @@ linters-settings:
min-complexity: 20
wrapcheck:
ignoreSigs:
- .Errorf(
- fmt.Errorf(
- errors.New(
- errors.Unwrap(
- .Wrap(
- .Wrapf(
- .WithMessage(
- errors.WrapIfWithDetails
- errors.WithDetails(
- errors.WrapWithDetails(
- errors.WrapIf(
- errors.NewWithDetails(

- errors.Join(
revive:
rules:
- name: dot-imports
arguments:
- allowedPackages: ["github.com/onsi/ginkgo/v2","github.com/onsi/gomega","github.com/onsi/gomega/gstruct"]
importas:
alias:
- pkg: k8s.io/apimachinery/pkg/api/errors
alias: k8serr
# Ensures that i.e. k8s.io/api/rbac/v1 is aliased as rbacv1
- pkg: k8s.io/api/(\w+)/(v[\w\d]+)
alias: $1$2
- pkg: github.com/openshift/api/(\w+)/(v[\w\d]+)
alias: $1$2
- pkg: (istio).io/api/(\w+)/(v[\w\d]+)
alias: $1$2$3
- pkg: (istio).io/client-go/pkg/apis/(\w+)/(v[\w\d]+)
alias: $1$2$3
ireturn:
allow:
# defaults https://golangci-lint.run/usage/linters/#ireturn
- anon
- error
- empty
- stdlib
# also allow generics
- generic
linters:
enable-all: true
disable:
Expand Down
14 changes: 7 additions & 7 deletions controllers/authorization/authorization_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package authorization

import (
"context"
"errors"
"fmt"

"github.com/go-logr/logr"
Expand All @@ -10,11 +11,10 @@ import (
"github.com/opendatahub-io/odh-platform/pkg/env"
"github.com/opendatahub-io/odh-platform/pkg/resource"
"github.com/opendatahub-io/odh-platform/pkg/spi"
istiosecv1beta1 "istio.io/client-go/pkg/apis/security/v1beta1"
apierrs "k8s.io/apimachinery/pkg/api/errors"
istiosecurityv1beta1 "istio.io/client-go/pkg/apis/security/v1beta1"
k8serr "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
k8serrs "k8s.io/apimachinery/pkg/util/errors"
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/builder"
"sigs.k8s.io/controller-runtime/pkg/client"
Expand Down Expand Up @@ -55,7 +55,7 @@ func (r *PlatformAuthorizationReconciler) Reconcile(ctx context.Context, req ctr
sourceRes.SetGroupVersionKind(r.authComponent.CustomResourceType)

if err := r.Client.Get(ctx, req.NamespacedName, sourceRes); err != nil {
if apierrs.IsNotFound(err) {
if k8serr.IsNotFound(err) {
r.log.Info("skipping reconcile. resource does not exist anymore", "resource", sourceRes.GroupVersionKind().String())

return ctrl.Result{}, nil
Expand All @@ -71,7 +71,7 @@ func (r *PlatformAuthorizationReconciler) Reconcile(ctx context.Context, req ctr
errs = append(errs, reconciler(ctx, sourceRes))
}

return ctrl.Result{}, k8serrs.NewAggregate(errs)
return ctrl.Result{}, errors.Join(errs...)
}

func (r *PlatformAuthorizationReconciler) SetupWithManager(mgr ctrl.Manager) error {
Expand All @@ -84,8 +84,8 @@ func (r *PlatformAuthorizationReconciler) SetupWithManager(mgr ctrl.Manager) err
},
}, builder.OnlyMetadata).
Owns(&authorinov1beta2.AuthConfig{}).
Owns(&istiosecv1beta1.AuthorizationPolicy{}).
Owns(&istiosecv1beta1.PeerAuthentication{}).
Owns(&istiosecurityv1beta1.AuthorizationPolicy{}).
Owns(&istiosecurityv1beta1.PeerAuthentication{}).
Complete(r)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import (
authorinov1beta2 "github.com/kuadrant/authorino/api/v1beta2"
"github.com/opendatahub-io/odh-platform/pkg/env"
"github.com/opendatahub-io/odh-platform/pkg/label"
apierrs "k8s.io/apimachinery/pkg/api/errors"
k8serr "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
"k8s.io/apimachinery/pkg/types"
Expand Down Expand Up @@ -42,7 +42,7 @@ func (r *PlatformAuthorizationReconciler) reconcileAuthConfig(ctx context.Contex
Namespace: desired.Namespace,
}, found)
if err != nil {
if apierrs.IsNotFound(err) {
if k8serr.IsNotFound(err) {
errCreate := r.Create(ctx, desired)
if client.IgnoreAlreadyExists(errCreate) != nil {
return fmt.Errorf("unable to create AuthConfig: %w", errCreate)
Expand Down
14 changes: 7 additions & 7 deletions controllers/authorization/authorization_reconcile_authpolicy.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@ import (
"github.com/opendatahub-io/odh-platform/pkg/label"
"istio.io/api/security/v1beta1"
istiotypev1beta1 "istio.io/api/type/v1beta1"
istiosecv1beta1 "istio.io/client-go/pkg/apis/security/v1beta1"
apierrs "k8s.io/apimachinery/pkg/api/errors"
istiosecurityv1beta1 "istio.io/client-go/pkg/apis/security/v1beta1"
k8serr "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
"k8s.io/apimachinery/pkg/types"
Expand All @@ -20,15 +20,15 @@ import (

func (r *PlatformAuthorizationReconciler) reconcileAuthPolicy(ctx context.Context, target *unstructured.Unstructured) error {
desired := createAuthorizationPolicy(r.authComponent.Ports, r.authComponent.WorkloadSelector, target)
found := &istiosecv1beta1.AuthorizationPolicy{}
found := &istiosecurityv1beta1.AuthorizationPolicy{}
justCreated := false

err := r.Get(ctx, types.NamespacedName{
Name: desired.Name,
Namespace: desired.Namespace,
}, found)
if err != nil {
if apierrs.IsNotFound(err) {
if k8serr.IsNotFound(err) {
errCreate := r.Create(ctx, desired)
if client.IgnoreAlreadyExists(errCreate) != nil {
return fmt.Errorf("unable to create AuthorizationPolicy: %w", errCreate)
Expand Down Expand Up @@ -66,8 +66,8 @@ func (r *PlatformAuthorizationReconciler) reconcileAuthPolicy(ctx context.Contex
return nil
}

func createAuthorizationPolicy(ports []string, workloadSelector map[string]string, target *unstructured.Unstructured) *istiosecv1beta1.AuthorizationPolicy {
policy := &istiosecv1beta1.AuthorizationPolicy{
func createAuthorizationPolicy(ports []string, workloadSelector map[string]string, target *unstructured.Unstructured) *istiosecurityv1beta1.AuthorizationPolicy {
policy := &istiosecurityv1beta1.AuthorizationPolicy{
ObjectMeta: metav1.ObjectMeta{
Name: target.GetName(),
Namespace: target.GetNamespace(),
Expand Down Expand Up @@ -112,7 +112,7 @@ func createAuthorizationPolicy(ports []string, workloadSelector map[string]strin
return policy
}

func CompareAuthPolicies(authzPolicy1, authzPolicy2 *istiosecv1beta1.AuthorizationPolicy) bool {
func CompareAuthPolicies(authzPolicy1, authzPolicy2 *istiosecurityv1beta1.AuthorizationPolicy) bool {
// .Spec contains MessageState from protobuf which has pragma.DoNotCopy (empty mutex slice)
// go vet complains about copying mutex when calling DeepEquals on passed variables, when it tries to access it.
// DeepCopy-ing solves this problem as it's using proto.Clone underneath. This implementation recreates mutex instead of
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@ import (
"github.com/opendatahub-io/odh-platform/pkg/label"
"istio.io/api/security/v1beta1"
istiotypev1beta1 "istio.io/api/type/v1beta1"
istiosecv1beta1 "istio.io/client-go/pkg/apis/security/v1beta1"
apierrs "k8s.io/apimachinery/pkg/api/errors"
istiosecurityv1beta1 "istio.io/client-go/pkg/apis/security/v1beta1"
k8serr "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
"k8s.io/apimachinery/pkg/types"
Expand All @@ -19,15 +19,15 @@ import (

func (r *PlatformAuthorizationReconciler) reconcilePeerAuthentication(ctx context.Context, target *unstructured.Unstructured) error {
desired := createPeerAuthentication(r.authComponent.WorkloadSelector, target)
found := &istiosecv1beta1.PeerAuthentication{}
found := &istiosecurityv1beta1.PeerAuthentication{}
justCreated := false

err := r.Get(ctx, types.NamespacedName{
Name: desired.Name,
Namespace: desired.Namespace,
}, found)
if err != nil {
if apierrs.IsNotFound(err) {
if k8serr.IsNotFound(err) {
errCreate := r.Create(ctx, desired)
if client.IgnoreAlreadyExists(errCreate) != nil {
return fmt.Errorf("unable to create PeerAuthentication: %w", errCreate)
Expand Down Expand Up @@ -65,8 +65,8 @@ func (r *PlatformAuthorizationReconciler) reconcilePeerAuthentication(ctx contex
return nil
}

func createPeerAuthentication(workloadSelector map[string]string, target *unstructured.Unstructured) *istiosecv1beta1.PeerAuthentication {
policy := &istiosecv1beta1.PeerAuthentication{
func createPeerAuthentication(workloadSelector map[string]string, target *unstructured.Unstructured) *istiosecurityv1beta1.PeerAuthentication {
policy := &istiosecurityv1beta1.PeerAuthentication{
ObjectMeta: metav1.ObjectMeta{
Name: target.GetName(),
Namespace: target.GetNamespace(),
Expand All @@ -87,7 +87,7 @@ func createPeerAuthentication(workloadSelector map[string]string, target *unstru
return policy
}

func ComparePeerAuthentication(peerAuth1, peerAuth2 *istiosecv1beta1.PeerAuthentication) bool {
func ComparePeerAuthentication(peerAuth1, peerAuth2 *istiosecurityv1beta1.PeerAuthentication) bool {
// .Spec contains MessageState from protobuf which has pragma.DoNotCopy (empty mutex slice)
// go vet complains about copying mutex when calling DeepEquals on passed variables, when it tries to access it.
// DeepCopy-ing solves this problem as it's using proto.Clone underneath. This implementation recreates mutex instead of
Expand Down
4 changes: 2 additions & 2 deletions pkg/schema/init.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ package schema

import (
authorino "github.com/kuadrant/authorino/api/v1beta2"
istiosecurity "istio.io/client-go/pkg/apis/security/v1beta1"
istiosecurityv1beta1 "istio.io/client-go/pkg/apis/security/v1beta1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
utilruntime "k8s.io/apimachinery/pkg/util/runtime"
Expand All @@ -14,5 +14,5 @@ func RegisterSchemes(s *runtime.Scheme) {
utilruntime.Must(metav1.AddMetaToScheme(s))
utilruntime.Must(clientgoscheme.AddToScheme(s))
utilruntime.Must(authorino.AddToScheme(s))
utilruntime.Must(istiosecurity.AddToScheme(s))
utilruntime.Must(istiosecurityv1beta1.AddToScheme(s))
}
6 changes: 3 additions & 3 deletions test/cluster/k8s_operations.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import (

. "github.com/onsi/gomega"
corev1 "k8s.io/api/core/v1"
apierrors "k8s.io/apimachinery/pkg/api/errors"
k8serr "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
"k8s.io/apimachinery/pkg/runtime/schema"
Expand Down Expand Up @@ -95,7 +95,7 @@ func (c *Cleaner) DeleteAll(objects ...client.Object) { //nolint:gocognit //reas
Eventually(func() metav1.StatusReason {
key := client.ObjectKeyFromObject(obj)
if err := c.client.Get(context.Background(), key, obj); err != nil {
return apierrors.ReasonForError(err)
return k8serr.ReasonForError(err)
}

return ""
Expand Down Expand Up @@ -147,7 +147,7 @@ func lookupNamespacedResources(clientset *kubernetes.Clientset) map[string]schem
}

func ignoreMethodNotAllowed(err error) error {
if apierrors.ReasonForError(err) == metav1.StatusReasonMethodNotAllowed {
if k8serr.ReasonForError(err) == metav1.StatusReasonMethodNotAllowed {
return nil
}

Expand Down

0 comments on commit 076faae

Please sign in to comment.