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

fix: Enable more linters #10

Merged
merged 6 commits into from
Mar 7, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
3 changes: 2 additions & 1 deletion .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -36,9 +36,10 @@ jobs:
# TODO coverage report

- name: Go linters
if: ${{ matrix.go-version == '1.21' }}
uses: golangci/golangci-lint-action@v4
with:
version: v1.55.2
version: v1.56.2
skip-pkg-cache: true

container-check:
Expand Down
12 changes: 8 additions & 4 deletions .golangci.yml
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
# Documentation reference https://github.com/golangci/golangci-lint/blob/v1.55.2/.golangci.reference.yml
# Documentation reference https://github.com/golangci/golangci-lint/blob/v1.56.2/.golangci.reference.yml
run:
mateusoliveira43 marked this conversation as resolved.
Show resolved Hide resolved
skip-dirs-use-default: false
modules-download-mode: readonly
Expand Down Expand Up @@ -61,6 +61,13 @@ linters-settings:
rules:
- name: line-length-limit
disabled: true
- name: function-length
disabled: true
# TODO remove
- name: cyclomatic
disabled: true
- name: cognitive-complexity
disabled: true
unparam:
check-exported: true

Expand Down Expand Up @@ -114,9 +121,6 @@ issues:
- linters:
- revive
text: "^add-constant: avoid magic numbers like '1', create a named constant for it$"
# - linters:
# - stylecheck
# text: "ST1000:|ST1020:|ST1021:|ST1022:"
max-issues-per-linter: 0
max-same-issues: 0

Expand Down
2 changes: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@ GOLANGCI_LINT = $(LOCALBIN)/golangci-lint-$(GOLANGCI_LINT_VERSION)
KUSTOMIZE_VERSION ?= v5.3.0
CONTROLLER_TOOLS_VERSION ?= v0.14.0
ENVTEST_VERSION ?= latest
GOLANGCI_LINT_VERSION ?= v1.55.2
GOLANGCI_LINT_VERSION ?= v1.56.2

.PHONY: kustomize
kustomize: $(KUSTOMIZE) ## Download kustomize locally if necessary.
Expand Down
1 change: 1 addition & 0 deletions cmd/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ See the License for the specific language governing permissions and
limitations under the License.
*/

// Entrypoint of the project
package main

import (
Expand Down
31 changes: 31 additions & 0 deletions internal/common/constant/constant.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
/*
Copyright 2024.

Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at

http://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

// Package constant contains all common constants used in the project
package constant

// Common labels for objects manipulated by the Non Admin Controller
// Labels should be used to identify the NAC backup
// Annotations on the other hand should be used to define ownership
// of the specific Object, such as Backup.
const (
OadpLabel = "openshift.io/oadp" // TODO import?
ManagedByLabel = "app.kubernetes.io/managed-by"
ManagedByLabelValue = "oadp-nac-controller" // TODO why not use same project name as in PROJECT file?
NabOriginNameAnnotation = "openshift.io/oadp-nab-origin-name"
NabOriginNamespaceAnnotation = "openshift.io/oadp-nab-origin-namespace"
NabOriginUUIDAnnotation = "openshift.io/oadp-nab-origin-uuid"
)
224 changes: 224 additions & 0 deletions internal/common/function/function.go
Copy link
Contributor Author

Choose a reason for hiding this comment

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

here we could separate in more files, but did not focused on this in this PR

Original file line number Diff line number Diff line change
@@ -0,0 +1,224 @@
/*
Copyright 2024.

Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at

http://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

// Package function contains all common functions used in the project
package function

import (
"context"
"crypto/sha1" //nolint:gosec // TODO remove
"encoding/hex"
"fmt"
"reflect"

"github.com/go-logr/logr"
velerov1api "github.com/vmware-tanzu/velero/pkg/apis/velero/v1"
"k8s.io/apimachinery/pkg/types"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/log"

nacv1alpha1 "github.com/migtools/oadp-non-admin/api/v1alpha1"
"github.com/migtools/oadp-non-admin/internal/common/constant"
)

const requiredAnnotationError = "backup does not have the required annotation '%s'"

// AddNonAdminLabels return a map with both the object labels and with the default Non Admin labels.
// If error occurs, a map with only the default Non Admin labels is returned
func AddNonAdminLabels(labels map[string]string) map[string]string {
defaultLabels := map[string]string{
constant.OadpLabel: "True",
constant.ManagedByLabel: constant.ManagedByLabelValue,
}

mergedLabels, err := mergeUniqueKeyTOfTMaps(defaultLabels, labels)
if err != nil {
// TODO logger
_, _ = fmt.Println("Error merging labels:", err)
// TODO break?
return defaultLabels
}
return mergedLabels
}

// AddNonAdminBackupAnnotations return a map with both the object annotations and with the default NonAdminBackup annotations.
// If error occurs, a map with only the default NonAdminBackup annotations is returned
func AddNonAdminBackupAnnotations(ownerNamespace string, ownerName string, ownerUUID string, existingAnnotations map[string]string) map[string]string {
// TODO could not receive object meta and get info from there?
defaultAnnotations := map[string]string{
constant.NabOriginNamespaceAnnotation: ownerNamespace,
constant.NabOriginNameAnnotation: ownerName,
constant.NabOriginUUIDAnnotation: ownerUUID,
}

mergedAnnotations, err := mergeUniqueKeyTOfTMaps(defaultAnnotations, existingAnnotations)
if err != nil {
// TODO logger
_, _ = fmt.Println("Error merging annotations:", err)
// TODO break?
return defaultAnnotations
}
return mergedAnnotations
}

// GetBackupSpecFromNonAdminBackup return BackupSpec object from NonAdminBackup spec, if no error occurs
func GetBackupSpecFromNonAdminBackup(nonAdminBackup *nacv1alpha1.NonAdminBackup) (*velerov1api.BackupSpec, error) {
if nonAdminBackup == nil {
return nil, fmt.Errorf("nonAdminBackup is nil")
}

// TODO check spec?

if nonAdminBackup.Spec.BackupSpec == nil {
return nil, fmt.Errorf("BackupSpec is nil")
}

// TODO: Additional validations, before continuing

return nonAdminBackup.Spec.BackupSpec.DeepCopy(), nil
}

// GenerateVeleroBackupName return generated name for Velero Backup object created from NonAdminBackup
func GenerateVeleroBackupName(namespace, nabName string) string {
// Calculate a hash of the name
hasher := sha1.New() //nolint:gosec // TODO use another tool
_, _ = hasher.Write([]byte(nabName))
const nameLength = 14
nameHash := hex.EncodeToString(hasher.Sum(nil))[:nameLength] // Take first 14 chars

// Generate the Velero backup name created from NAB
veleroBackupName := fmt.Sprintf("nab-%s-%s", namespace, nameHash)

const characterLimit = 253
const occupiedSize = 4
// Ensure the name is within the character limit
if len(veleroBackupName) > characterLimit {
// Truncate the namespace if necessary
maxNamespaceLength := characterLimit - len(nameHash) - occupiedSize // Account for "nab-" and "-" TODO should not be 5?
if len(namespace) > maxNamespaceLength {
namespace = namespace[:maxNamespaceLength]
}
veleroBackupName = fmt.Sprintf("nab-%s-%s", namespace, nameHash)
}

return veleroBackupName
}

// UpdateNonAdminBackupFromVeleroBackup update, if necessary, NonAdminBackup object fields related to referenced Velero Backup object, if no error occurs
func UpdateNonAdminBackupFromVeleroBackup(ctx context.Context, r client.Client, logger logr.Logger, nab *nacv1alpha1.NonAdminBackup, veleroBackup *velerov1api.Backup) error {
// Make a copy of the current status for comparison
oldStatus := nab.Spec.BackupStatus.DeepCopy()
oldSpec := nab.Spec.BackupSpec.DeepCopy()

// Update the status & spec
nab.Spec.BackupStatus = &veleroBackup.Status
nab.Spec.BackupSpec = &veleroBackup.Spec

if reflect.DeepEqual(oldStatus, nab.Spec.BackupStatus) && reflect.DeepEqual(oldSpec, nab.Spec.BackupSpec) {
// No change, no need to update
logger.V(1).Info("NonAdminBackup status and spec is already up to date")
return nil
}

if err := r.Update(ctx, nab); err != nil {
logger.Error(err, "Failed to update NonAdminBackup")
return err
}

return nil
}

// CheckVeleroBackupLabels return true if Velero Backup object has required Non Admin labels, false otherwise
func CheckVeleroBackupLabels(backup *velerov1api.Backup) bool {
// TODO also need to check for constant.OadpLabel label?
labels := backup.GetLabels()
value, exists := labels[constant.ManagedByLabel]
return exists && value == constant.ManagedByLabelValue
}

// TODO not used

// GetNonAdminBackupFromVeleroBackup return referenced NonAdminBackup object from Velero Backup object, if no error occurs
func GetNonAdminBackupFromVeleroBackup(ctx context.Context, clientInstance client.Client, backup *velerov1api.Backup) (*nacv1alpha1.NonAdminBackup, error) {
// Check if the backup has the required annotations to identify the associated NonAdminBackup object
logger := log.FromContext(ctx)

annotations := backup.GetAnnotations()

annotationsStr := fmt.Sprintf("%v", annotations)
logger.V(1).Info("Velero Backup Annotations", "annotations", annotationsStr)

if annotations == nil {
return nil, fmt.Errorf("backup has no annotations")
}

nabOriginNamespace, ok := annotations[constant.NabOriginNamespaceAnnotation]
if !ok {
return nil, fmt.Errorf(requiredAnnotationError, constant.NabOriginNamespaceAnnotation)
}

nabOriginName, ok := annotations[constant.NabOriginNameAnnotation]
if !ok {
return nil, fmt.Errorf(requiredAnnotationError, constant.NabOriginNameAnnotation)
}

nonAdminBackupKey := types.NamespacedName{
Namespace: nabOriginNamespace,
Name: nabOriginName,
}

nonAdminBackup := &nacv1alpha1.NonAdminBackup{}
err := clientInstance.Get(ctx, nonAdminBackupKey, nonAdminBackup)
if err != nil {
return nil, fmt.Errorf("failed to fetch NonAdminBackup object: %v", err)
}

nabOriginUUID, ok := annotations[constant.NabOriginUUIDAnnotation]
if !ok {
return nil, fmt.Errorf(requiredAnnotationError, constant.NabOriginUUIDAnnotation)
}
// Ensure UID matches
if nonAdminBackup.ObjectMeta.UID != types.UID(nabOriginUUID) {
return nil, fmt.Errorf("UID from annotation does not match UID of fetched NonAdminBackup object")
}

return nonAdminBackup, nil
}

// TODO import?
// Similar to as pkg/common/common.go:AppendUniqueKeyTOfTMaps from github.com/openshift/oadp-operator
func mergeUniqueKeyTOfTMaps[T comparable](userMap ...map[T]T) (map[T]T, error) {
mateusoliveira43 marked this conversation as resolved.
Show resolved Hide resolved
var base map[T]T
for i, mapElements := range userMap {
if mapElements == nil {
continue
}
if base == nil {
base = make(map[T]T)
}
for k, v := range mapElements {
existingValue, found := base[k]
if found {
if existingValue != v {
return nil, fmt.Errorf("conflicting key %v with value %v in map %d may not override %v", k, v, i, existingValue)
}
} else {
base[k] = v
}
}
}
return base, nil
}
Loading
Loading