Skip to content

Commit

Permalink
Enable stylecheck linter and resolve found issues.
Browse files Browse the repository at this point in the history
Signed-off-by: Xun Jiang <[email protected]>
  • Loading branch information
Xun Jiang committed Apr 25, 2023
1 parent 980106d commit 3175fcf
Show file tree
Hide file tree
Showing 39 changed files with 214 additions and 227 deletions.
1 change: 1 addition & 0 deletions golangci.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -298,6 +298,7 @@ linters:
- gosec
- govet
- misspell
- stylecheck
- typecheck
- unparam
- unused
Expand Down
8 changes: 4 additions & 4 deletions hack/release-tools/chk_version.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,19 +29,19 @@ import (
// minor
// patch
// prerelease (this will be alpha/beta/rc followed by a ".", followed by 1 or more digits (alpha.5)
var release_regex *regexp.Regexp = regexp.MustCompile(`^v(?P<major>[[:digit:]]+)\.(?P<minor>[[:digit:]]+)\.(?P<patch>[[:digit:]]+)(-{1}(?P<prerelease>(alpha|beta|rc)\.[[:digit:]]+))*`)
var releaseRegex = regexp.MustCompile(`^v(?P<major>[[:digit:]]+)\.(?P<minor>[[:digit:]]+)\.(?P<patch>[[:digit:]]+)(-{1}(?P<prerelease>(alpha|beta|rc)\.[[:digit:]]+))*`)

// This small program exists because checking the VELERO_VERSION rules in bash is difficult, and difficult to test for correctness.
// Calling it with --verify will verify whether or not the VELERO_VERSION environment variable is a valid version string, without parsing for its components.
// Calling it without --verify will try to parse the version into its component pieces.
func main() {
velero_version := os.Getenv("VELERO_VERSION")
veleroVersion := os.Getenv("VELERO_VERSION")

submatches := reSubMatchMap(release_regex, velero_version)
submatches := reSubMatchMap(releaseRegex, veleroVersion)

// Didn't match the regex, exit.
if len(submatches) == 0 {
fmt.Printf("VELERO_VERSION of %s was not valid. Please correct the value and retry.", velero_version)
fmt.Printf("VELERO_VERSION of %s was not valid. Please correct the value and retry.", veleroVersion)
os.Exit(1)
}

Expand Down
2 changes: 1 addition & 1 deletion hack/release-tools/chk_version_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ func TestRegexMatching(t *testing.T) {
for _, test := range tests {
name := fmt.Sprintf("Testing version string %s", test.version)
t.Run(name, func(t *testing.T) {
results := reSubMatchMap(release_regex, test.version)
results := reSubMatchMap(releaseRegex, test.version)

if len(results) == 0 && test.expectMatch {
t.Fail()
Expand Down
18 changes: 9 additions & 9 deletions internal/resourcepolicies/resource_policies.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ func unmarshalResourcePolicies(yamlData *string) (*resourcePolicies, error) {
}
}

func (policies *Policies) buildPolicy(resPolicies *resourcePolicies) error {
func (p *Policies) buildPolicy(resPolicies *resourcePolicies) error {
for _, vp := range resPolicies.VolumePolicies {
con, err := unmarshalVolConditions(vp.Conditions)
if err != nil {
Expand All @@ -64,18 +64,18 @@ func (policies *Policies) buildPolicy(resPolicies *resourcePolicies) error {
if err != nil {
return errors.WithStack(err)
}
var p volPolicy
p.action = vp.Action
p.conditions = append(p.conditions, &capacityCondition{capacity: *volCap})
p.conditions = append(p.conditions, &storageClassCondition{storageClass: con.StorageClass})
p.conditions = append(p.conditions, &nfsCondition{nfs: con.NFS})
p.conditions = append(p.conditions, &csiCondition{csi: con.CSI})
policies.volumePolicies = append(policies.volumePolicies, p)
var volP volPolicy
volP.action = vp.Action
volP.conditions = append(volP.conditions, &capacityCondition{capacity: *volCap})
volP.conditions = append(volP.conditions, &storageClassCondition{storageClass: con.StorageClass})
volP.conditions = append(volP.conditions, &nfsCondition{nfs: con.NFS})
volP.conditions = append(volP.conditions, &csiCondition{csi: con.CSI})
p.volumePolicies = append(p.volumePolicies, volP)
}

// Other resource policies

policies.version = resPolicies.Version
p.version = resPolicies.Version
return nil
}

Expand Down
4 changes: 1 addition & 3 deletions pkg/backup/backup_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,6 @@ import (
vsv1 "github.com/vmware-tanzu/velero/pkg/plugin/velero/volumesnapshotter/v1"
"github.com/vmware-tanzu/velero/pkg/podvolume"
"github.com/vmware-tanzu/velero/pkg/test"
testutil "github.com/vmware-tanzu/velero/pkg/test"
kubeutil "github.com/vmware-tanzu/velero/pkg/util/kube"
"github.com/vmware-tanzu/velero/pkg/volume"
)
Expand Down Expand Up @@ -107,7 +106,6 @@ func TestBackedUpItemsMatchesTarballContents(t *testing.T) {
if item.namespace != "" {
fileWithVersion = fileWithVersion + "/v1-preferredversion/" + "namespaces/" + item.namespace
} else {
file = file + "/cluster"
fileWithVersion = fileWithVersion + "/v1-preferredversion" + "/cluster"
}
fileWithVersion = fileWithVersion + "/" + item.name + ".json"
Expand Down Expand Up @@ -2845,7 +2843,7 @@ func TestBackupWithHooks(t *testing.T) {
h = newHarness(t)
req = &Request{Backup: tc.backup}
backupFile = bytes.NewBuffer([]byte{})
podCommandExecutor = new(testutil.MockPodCommandExecutor)
podCommandExecutor = new(test.MockPodCommandExecutor)
)

h.backupper.podCommandExecutor = podCommandExecutor
Expand Down
10 changes: 5 additions & 5 deletions pkg/builder/customresourcedefinition_v1beta1_builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,17 +48,17 @@ func (c *CustomResourceDefinitionV1Beta1Builder) Condition(cond apiextv1beta1.Cu
}

// Result returns the built CustomResourceDefinition.
func (b *CustomResourceDefinitionV1Beta1Builder) Result() *apiextv1beta1.CustomResourceDefinition {
return b.object
func (c *CustomResourceDefinitionV1Beta1Builder) Result() *apiextv1beta1.CustomResourceDefinition {
return c.object
}

// ObjectMeta applies functional options to the CustomResourceDefinition's ObjectMeta.
func (b *CustomResourceDefinitionV1Beta1Builder) ObjectMeta(opts ...ObjectMetaOpt) *CustomResourceDefinitionV1Beta1Builder {
func (c *CustomResourceDefinitionV1Beta1Builder) ObjectMeta(opts ...ObjectMetaOpt) *CustomResourceDefinitionV1Beta1Builder {
for _, opt := range opts {
opt(b.object)
opt(c.object)
}

return b
return c
}

// CustomResourceDefinitionV1Beta1ConditionBuilder builds CustomResourceDefinitionV1Beta1Condition objects.
Expand Down
30 changes: 15 additions & 15 deletions pkg/builder/v1_customresourcedefinition_builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -98,8 +98,8 @@ func (c *V1CustomResourceDefinitionConditionBuilder) Status(cs apiextv1.Conditio
}

// Result returns the built CustomResourceDefinitionCondition.
func (b *V1CustomResourceDefinitionConditionBuilder) Result() apiextv1.CustomResourceDefinitionCondition {
return b.object
func (c *V1CustomResourceDefinitionConditionBuilder) Result() apiextv1.CustomResourceDefinitionCondition {
return c.object
}

// V1CustomResourceDefinitionVersionBuilder builds CustomResourceDefinitionVersion objects.
Expand All @@ -115,26 +115,26 @@ func ForV1CustomResourceDefinitionVersion(name string) *V1CustomResourceDefiniti
}

// Served sets the Served field on a CustomResourceDefinitionVersion.
func (b *V1CustomResourceDefinitionVersionBuilder) Served(s bool) *V1CustomResourceDefinitionVersionBuilder {
b.object.Served = s
return b
func (c *V1CustomResourceDefinitionVersionBuilder) Served(s bool) *V1CustomResourceDefinitionVersionBuilder {
c.object.Served = s
return c
}

// Storage sets the Storage field on a CustomResourceDefinitionVersion.
func (b *V1CustomResourceDefinitionVersionBuilder) Storage(s bool) *V1CustomResourceDefinitionVersionBuilder {
b.object.Storage = s
return b
func (c *V1CustomResourceDefinitionVersionBuilder) Storage(s bool) *V1CustomResourceDefinitionVersionBuilder {
c.object.Storage = s
return c
}

func (b *V1CustomResourceDefinitionVersionBuilder) Schema(s *apiextv1.JSONSchemaProps) *V1CustomResourceDefinitionVersionBuilder {
if b.object.Schema == nil {
b.object.Schema = new(apiextv1.CustomResourceValidation)
func (c *V1CustomResourceDefinitionVersionBuilder) Schema(s *apiextv1.JSONSchemaProps) *V1CustomResourceDefinitionVersionBuilder {
if c.object.Schema == nil {
c.object.Schema = new(apiextv1.CustomResourceValidation)
}
b.object.Schema.OpenAPIV3Schema = s
return b
c.object.Schema.OpenAPIV3Schema = s
return c
}

// Result returns the built CustomResourceDefinitionVersion.
func (b *V1CustomResourceDefinitionVersionBuilder) Result() apiextv1.CustomResourceDefinitionVersion {
return b.object
func (c *V1CustomResourceDefinitionVersionBuilder) Result() apiextv1.CustomResourceDefinitionVersion {
return c.object
}
2 changes: 1 addition & 1 deletion pkg/cmd/cli/backup/describe.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ func NewDescribeCommand(f client.Factory, use string) *cobra.Command {
cmd.CheckError(err)

if outputFormat != "plaintext" && outputFormat != "json" {
cmd.CheckError(fmt.Errorf("Invalid output format '%s'. Valid value are 'plaintext, json'", outputFormat))
cmd.CheckError(fmt.Errorf("invalid output format '%s'. valid value are 'plaintext, json'", outputFormat))
}

var backups *velerov1api.BackupList
Expand Down
2 changes: 1 addition & 1 deletion pkg/cmd/cli/delete_options.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ func (o *DeleteOptions) Complete(f client.Factory, args []string) error {
// Validate validates the fields of the DeleteOptions struct.
func (o *DeleteOptions) Validate(c *cobra.Command, f client.Factory, args []string) error {
if o.Client == nil {
return errors.New("Velero client is not set; unable to proceed")
return errors.New("velero client is not set; unable to proceed")
}

return o.SelectOptions.Validate()
Expand Down
3 changes: 1 addition & 2 deletions pkg/cmd/cli/nodeagent/server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ import (

"github.com/stretchr/testify/assert"
corev1 "k8s.io/api/core/v1"
v1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/client-go/kubernetes/fake"

Expand Down Expand Up @@ -51,7 +50,7 @@ func Test_validatePodVolumesHostPath(t *testing.T) {
name: "no error when pod volumes are present and there are mirror pods",
pods: []*corev1.Pod{
builder.ForPod("foo", "bar").ObjectMeta(builder.WithUID("foo")).Result(),
builder.ForPod("zoo", "raz").ObjectMeta(builder.WithUID("zoo"), builder.WithAnnotations(v1.MirrorPodAnnotationKey, "baz")).Result(),
builder.ForPod("zoo", "raz").ObjectMeta(builder.WithUID("zoo"), builder.WithAnnotations(corev1.MirrorPodAnnotationKey, "baz")).Result(),
},
dirs: []string{"foo", "baz"},
wantErr: false,
Expand Down
3 changes: 1 addition & 2 deletions pkg/cmd/cli/serverstatus/server_status.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ import (

"github.com/pkg/errors"
"k8s.io/apimachinery/pkg/util/wait"
"sigs.k8s.io/controller-runtime/pkg/client"
kbclient "sigs.k8s.io/controller-runtime/pkg/client"

velerov1api "github.com/vmware-tanzu/velero/pkg/apis/velero/v1"
Expand All @@ -48,7 +47,7 @@ func (g *DefaultServerStatusGetter) GetServerStatus(kbClient kbclient.Client) (*
ctx, cancel := context.WithCancel(g.Context)
defer cancel()

key := client.ObjectKey{Name: created.Name, Namespace: g.Namespace}
key := kbclient.ObjectKey{Name: created.Name, Namespace: g.Namespace}
checkFunc := func() {
updated := &velerov1api.ServerStatusRequest{}
if err := kbClient.Get(ctx, key, updated); err != nil {
Expand Down
6 changes: 3 additions & 3 deletions pkg/cmd/util/downloadrequest/downloadrequest.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,11 +60,11 @@ func Stream(ctx context.Context, kbClient kbclient.Client, namespace, name strin

key := kbclient.ObjectKey{Name: created.Name, Namespace: namespace}
timeStreamFirstCheck := time.Now()
downloadUrlTimeout := false
downloadURLTimeout := false
checkFunc := func() {
// if timeout has been reached, cancel request
if time.Now().After(timeStreamFirstCheck.Add(timeout)) {
downloadUrlTimeout = true
downloadURLTimeout = true
cancel()
}
updated := &velerov1api.DownloadRequest{}
Expand All @@ -85,7 +85,7 @@ func Stream(ctx context.Context, kbClient kbclient.Client, namespace, name strin
}

wait.Until(checkFunc, 25*time.Millisecond, ctx.Done())
if downloadUrlTimeout {
if downloadURLTimeout {
return ErrDownloadRequestDownloadURLTimeout
}

Expand Down
6 changes: 3 additions & 3 deletions pkg/cmd/util/output/describe.go
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ func NewStructuredDescriber(format string) *StructuredDescriber {
func DescribeInSF(fn func(d *StructuredDescriber), format string) string {
d := NewStructuredDescriber(format)
fn(d)
return d.JsonEncode()
return d.JSONEncode()
}

// Describe adds all types of argument to d.output.
Expand All @@ -167,8 +167,8 @@ func (d *StructuredDescriber) DescribeMetadata(metadata metav1.ObjectMeta) {
d.Describe("metadata", metadataInfo)
}

// JsonEncode encodes d.output to json
func (d *StructuredDescriber) JsonEncode() string {
// JSONEncode encodes d.output to json
func (d *StructuredDescriber) JSONEncode() string {
byteBuffer := &bytes.Buffer{}
encoder := json.NewEncoder(byteBuffer)
encoder.SetEscapeHTML(false)
Expand Down
5 changes: 2 additions & 3 deletions pkg/controller/backup_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ import (
"github.com/sirupsen/logrus"
"golang.org/x/sync/errgroup"
corev1api "k8s.io/api/core/v1"
v1 "k8s.io/api/core/v1"
apierrors "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/labels"
Expand Down Expand Up @@ -468,7 +467,7 @@ func (b *backupReconciler) prepareBackupRequest(backup *velerov1api.Backup, logg
}

if request.Spec.ResourcePolicy != nil && request.Spec.ResourcePolicy.Kind == resourcepolicies.ConfigmapRefType {
policiesConfigmap := &v1.ConfigMap{}
policiesConfigmap := &corev1api.ConfigMap{}
err := b.kbClient.Get(context.Background(), kbclient.ObjectKey{Namespace: request.Namespace, Name: request.Spec.ResourcePolicy.Name}, policiesConfigmap)
if err != nil {
request.Status.ValidationErrors = append(request.Status.ValidationErrors, fmt.Sprintf("failed to get resource policies %s/%s configmap with err %v", request.Namespace, request.Spec.ResourcePolicy.Name, err))
Expand Down Expand Up @@ -1125,7 +1124,7 @@ func (b *backupReconciler) recreateVolumeSnapshotContent(vsc snapshotv1api.Volum
// validation webhook will check whether name and namespace are nil.
// external-snapshotter needs Source pointing to snapshot and VolumeSnapshot
// reference's UID to nil to determine the VolumeSnapshotContent is deletable.
vsc.Spec.VolumeSnapshotRef = v1.ObjectReference{
vsc.Spec.VolumeSnapshotRef = corev1api.ObjectReference{
APIVersion: snapshotv1api.SchemeGroupVersion.String(),
Kind: "VolumeSnapshot",
Namespace: "ns-" + string(vsc.UID),
Expand Down
28 changes: 14 additions & 14 deletions pkg/controller/backup_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -298,8 +298,8 @@ func Test_prepareBackupRequest_BackupStorageLocation(t *testing.T) {
name string
backup *velerov1api.Backup
backupLocationNameInBackup string
backupLocationInApiServer *velerov1api.BackupStorageLocation
defaultBackupLocationInApiServer *velerov1api.BackupStorageLocation
backupLocationInAPIServer *velerov1api.BackupStorageLocation
defaultBackupLocationInAPIServer *velerov1api.BackupStorageLocation
expectedBackupLocation string
expectedSuccess bool
expectedValidationError string
Expand All @@ -308,35 +308,35 @@ func Test_prepareBackupRequest_BackupStorageLocation(t *testing.T) {
name: "BackupLocation is specified in backup CR'spec and it can be found in ApiServer",
backup: builder.ForBackup("velero", "backup-1").Result(),
backupLocationNameInBackup: "test-backup-location",
backupLocationInApiServer: builder.ForBackupStorageLocation("velero", "test-backup-location").Result(),
defaultBackupLocationInApiServer: builder.ForBackupStorageLocation("velero", "default-location").Result(),
backupLocationInAPIServer: builder.ForBackupStorageLocation("velero", "test-backup-location").Result(),
defaultBackupLocationInAPIServer: builder.ForBackupStorageLocation("velero", "default-location").Result(),
expectedBackupLocation: "test-backup-location",
expectedSuccess: true,
},
{
name: "BackupLocation is specified in backup CR'spec and it can't be found in ApiServer",
backup: builder.ForBackup("velero", "backup-1").Result(),
backupLocationNameInBackup: "test-backup-location",
backupLocationInApiServer: nil,
defaultBackupLocationInApiServer: nil,
backupLocationInAPIServer: nil,
defaultBackupLocationInAPIServer: nil,
expectedSuccess: false,
expectedValidationError: "an existing backup storage location wasn't specified at backup creation time and the default 'test-backup-location' wasn't found. Please address this issue (see `velero backup-location -h` for options) and create a new backup. Error: backupstoragelocations.velero.io \"test-backup-location\" not found",
},
{
name: "Using default BackupLocation and it can be found in ApiServer",
backup: builder.ForBackup("velero", "backup-1").Result(),
backupLocationNameInBackup: "",
backupLocationInApiServer: builder.ForBackupStorageLocation("velero", "test-backup-location").Result(),
defaultBackupLocationInApiServer: builder.ForBackupStorageLocation("velero", "default-location").Result(),
backupLocationInAPIServer: builder.ForBackupStorageLocation("velero", "test-backup-location").Result(),
defaultBackupLocationInAPIServer: builder.ForBackupStorageLocation("velero", "default-location").Result(),
expectedBackupLocation: defaultBackupLocation,
expectedSuccess: true,
},
{
name: "Using default BackupLocation and it can't be found in ApiServer",
backup: builder.ForBackup("velero", "backup-1").Result(),
backupLocationNameInBackup: "",
backupLocationInApiServer: nil,
defaultBackupLocationInApiServer: nil,
backupLocationInAPIServer: nil,
defaultBackupLocationInAPIServer: nil,
expectedSuccess: false,
expectedValidationError: fmt.Sprintf("an existing backup storage location wasn't specified at backup creation time and the server default '%s' doesn't exist. Please address this issue (see `velero backup-location -h` for options) and create a new backup. Error: backupstoragelocations.velero.io \"%s\" not found", defaultBackupLocation, defaultBackupLocation),
},
Expand All @@ -353,11 +353,11 @@ func Test_prepareBackupRequest_BackupStorageLocation(t *testing.T) {

// objects that should init with client
objects := make([]runtime.Object, 0)
if test.backupLocationInApiServer != nil {
objects = append(objects, test.backupLocationInApiServer)
if test.backupLocationInAPIServer != nil {
objects = append(objects, test.backupLocationInAPIServer)
}
if test.defaultBackupLocationInApiServer != nil {
objects = append(objects, test.defaultBackupLocationInApiServer)
if test.defaultBackupLocationInAPIServer != nil {
objects = append(objects, test.defaultBackupLocationInAPIServer)
}
fakeClient := velerotest.NewFakeControllerRuntimeClient(t, objects...)

Expand Down
Loading

0 comments on commit 3175fcf

Please sign in to comment.