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

chore: fix code smell (#208) #210

Merged
merged 1 commit into from
May 25, 2022
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
129 changes: 3 additions & 126 deletions Makefile
Original file line number Diff line number Diff line change
@@ -1,127 +1,4 @@
# Produce CRDs that work back to Kubernetes 1.11 (no version conversion)
CRD_OPTIONS ?= "crd"
include base.mk

LOCAL ?=

# Get the currently used golang install path (in GOPATH/bin, unless GOBIN is set)
ifeq (,$(shell go env GOBIN))
GOBIN=$(shell go env GOPATH)/bin
else
GOBIN=$(shell go env GOBIN)
endif

# Setting SHELL to bash allows bash commands to be executed by recipes.
# This is a requirement for 'setup-envtest.sh' in the test target.
# Options are set to exit when a recipe line exits non-zero or a piped command fails.
SHELL = /usr/bin/env bash -o pipefail
.SHELLFLAGS = -ec

all: lint test

##@ General

# The help target prints out all targets with their descriptions organized
# beneath their categories. The categories are represented by '##@' and the
# target descriptions by '##'. The awk commands is responsible for reading the
# entire set of makefiles included in this invocation, looking for lines of the
# file as xyz: ## something, and then pretty-format the target and help. Then,
# if there's a line with ##@ something, that gets pretty-printed as a category.
# More info on the usage of ANSI control characters for terminal formatting:
# https://en.wikipedia.org/wiki/ANSI_escape_code#SGR_parameters
# More info on the awk command:
# http://linuxcommand.org/lc3_adv_awk.php

help: ## Display this help.
@awk 'BEGIN {FS = ":.*##"; printf "\nUsage:\n make \033[36m<target>\033[0m\n"} /^[a-zA-Z_0-9-]+:.*?##/ { printf " \033[36m%-15s\033[0m %s\n", $$1, $$2 } /^##@/ { printf "\n\033[1m%s\033[0m\n", substr($$0, 5) } ' $(MAKEFILE_LIST)

##@ Development

fmt: ## Run go fmt against code.
go fmt ./...

vet: ## Run go vet against code.
go vet ./...

lint: golangcilint ## Run golangci-lint against code.
$(GOLANGCILINT) run

ENVTEST_ASSETS_DIR=$(shell pwd)/testbin
test: fmt vet goimports ## Run tests.
mkdir -p ${ENVTEST_ASSETS_DIR}
test -f ${ENVTEST_ASSETS_DIR}/setup-envtest.sh || curl -sSLo ${ENVTEST_ASSETS_DIR}/setup-envtest.sh https://raw.githubusercontent.com/kubernetes-sigs/controller-runtime/v0.8.3/hack/setup-envtest.sh
source ${ENVTEST_ASSETS_DIR}/setup-envtest.sh; fetch_envtest_tools $(ENVTEST_ASSETS_DIR); setup_envtest_env $(ENVTEST_ASSETS_DIR); go test ./... -coverprofile cover.out

generate: controller-gen ## Generate code containing DeepCopy, DeepCopyInto, and DeepCopyObject method implementations.
$(CONTROLLER_GEN) object:headerFile="hack/boilerplate.go.txt" paths="./..."

##@ Setup

CONTROLLER_GEN = $(shell pwd)/bin/controller-gen
controller-gen: ## Download controller-gen locally if necessary.
## this is a necessary evil already reported by knative community https://github.com/kubernetes-sigs/controller-tools/ issue 560
## once the issue is fixed we can move to use the original package. the original line uses go-get-tools with sigs.k8s.io/controller-tools/cmd/[email protected]
$(call go-get-fork,$(CONTROLLER_GEN),https://github.com/katanomi/controller-tools,cmd/controller-gen,controller-gen)

KUSTOMIZE = $(shell pwd)/bin/kustomize
kustomize: ## Download kustomize locally if necessary.
$(call go-get-tool,$(KUSTOMIZE),sigs.k8s.io/kustomize/kustomize/[email protected])

KO = $(shell pwd)/bin/ko
ko: ## Download ko locally if necessary.
$(call go-get-tool,$(KO),github.com/google/[email protected])

GOIMPORTS = $(shell pwd)/bin/goimports
goimports: ## Download goimports locally if necessary.
$(call go-get-tool,$(GOIMPORTS),golang.org/x/tools/cmd/goimports)
$(GOIMPORTS) -w -l $(shell find . -path '.git' -prune -path './vendor' -prune -o -name '*.pb.go' -prune -o -type f -name '*.go' -print)

GINKGO = $(shell pwd)/bin/ginkgo
ginkgo: ## Download ginkgo locally if necessary
$(call go-get-tool,$(GINKGO),github.com/onsi/ginkgo/[email protected])

GOMOCK = $(shell pwd)/bin/mockgen
gomock: ## Download gomock locally if necessary.
$(call go-get-tool,$(GOMOCK),github.com/golang/mock/[email protected])

GOLANGCILINT = $(shell pwd)/bin/golangci-lint
golangcilint: ## Download golangci-lint locally if necessary
$(call go-get-tool,$(GOLANGCILINT),github.com/golangci/golangci-lint/cmd/[email protected])

# go-get-tool will 'go get' 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 get $(2) ;\
rm -rf $$TMP_DIR ;\
}
endef

# go-get-fork is a "go-get-tool" like command to get temporary module forks.
define go-get-fork
@[ -f $(1) ] || { \
set -e ;\
TMP_DIR=$$(mktemp -d) ;\
cd $$TMP_DIR ;\
echo "Cloning $(2)" ;\
git clone $(2) $(4) ;\
cd $(4) ;\
GOBIN=$(PROJECT_DIR)/bin go install ./$(3);\
rm -rf $$TMP_DIR ;\
}
endef

# installyaml will check if a given namespace is present, if not will apply a yaml file and wait for a deployment to rollout
define installyaml
kubectl get ns $(1) > /dev/null ;\
EXIT_CODE=$$?;\
[ "$$EXIT_CODE" == "0" ] || { \
set -e ;\
kubectl apply -f $(2) ;\
kubectl -n $(1) rollout status deploy/$(3) --timeout=10m ;\
}
endef
manifests: controller-gen ##@Development Generate WebhookConfiguration, ClusterRole and CustomResourceDefinition objects.
# $(CONTROLLER_GEN) rbac:roleName=pkg paths="./..."
6 changes: 3 additions & 3 deletions apis/data/v1alpha1/labels_annotations.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ limitations under the License.

package v1alpha1

// storage resource type
// StorageResourceType storage resource type
type StorageResourceType string

const (
Expand All @@ -28,7 +28,7 @@ func (s StorageResourceType) String() string {
return string(s)
}

// Payload type
// PayloadType payload type
type PayloadType string

const (
Expand All @@ -43,7 +43,7 @@ func (p PayloadType) String() string {
return string(p)
}

// backend storage type
// BackendType backend storage type
type BackendType string

const (
Expand Down
2 changes: 1 addition & 1 deletion apis/data/v1alpha1/zz_generated.deepcopy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

7 changes: 5 additions & 2 deletions apis/meta/v1alpha1/auth_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ type AuthCheckOptions struct {
}

// AuthCheck consists of result for an auth check request
// +k8s:deepcopy-gen=false
type AuthCheck struct {
metav1.TypeMeta `json:",inline"`
Spec *AuthCheckSpec `json:"spec,omitempty"`
Expand All @@ -54,6 +55,8 @@ type AuthCheckSpec struct {
Version string `json:"version,omitempty"`
}

// +k8s:deepcopy-gen=false

type AuthCheckStatus struct {
// Allowed describes if the headers used where accepted or not by the integrated system.
// `True` when accepted,
Expand Down Expand Up @@ -85,15 +88,15 @@ const (
NeedsAuthorizationAuthCheckReason = "NeedsAuthorization"
)

// +k8s:deepcopy-gen=false
// AuthToken access token request response
// +k8s:deepcopy-gen=false
type AuthToken struct {
metav1.TypeMeta `json:",inline"`
Status AuthTokenStatus `json:"status"`
}

// +k8s:deepcopy-gen=false
// AuthTokenStatus access token request response status
// +k8s:deepcopy-gen=false
type AuthTokenStatus struct {
// AccessTokenKey store the key for accessToken it is mainly for git clone as userName
AccessTokenKey string `json:"accessTokenKey"`
Expand Down
4 changes: 2 additions & 2 deletions apis/meta/v1alpha1/buildmetadata_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ const (
BuildMetadataKey = "builds.katanomi.dev/buildrun"
)

// This structure is a derivative of buildrun and is used for artifacts to record build information.
// BuildMetaData this structure is a derivative of buildrun and is used for artifacts to record build information.
type BuildMetaData struct {
metav1.TypeMeta `json:",inline"`
metav1.ObjectMeta `json:"metadata,omitempty"`
Expand Down Expand Up @@ -59,7 +59,7 @@ type BuildRunGitStatus struct {
Branch *BuildGitBranchStatus `json:"branch,omitempty"`
}

// GitBranchStatus represent branch status of build run
// BuildGitBranchStatus represent branch status of build run
type BuildGitBranchStatus struct {
// Name of git branch
Name string `json:"name"`
Expand Down
3 changes: 2 additions & 1 deletion apis/meta/v1alpha1/code_quality_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ type CodeQualityBranch struct {
Metrics map[string]CodeQualityAnalyzeMetric `json:"metrics"`
}

// CodeQualityAnalyzeResult present CodeQualityProject analyze result
// CodeQualityAnalyzeMetric present CodeQualityProject analyze result
type CodeQualityAnalyzeMetric struct {
// Value defines the value of this metric
Value string `json:"value"`
Expand Down Expand Up @@ -84,6 +84,7 @@ type CodeQualityTaskOption struct {
PullRequest string `json:"pullRequest"`
}

// CodeQualityLineChartOption code quality line chart option
// +k8s:deepcopy-gen=false
type CodeQualityLineChartOption struct {
CodeQualityBaseOption
Expand Down
4 changes: 2 additions & 2 deletions apis/meta/v1alpha1/condition.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,11 +51,11 @@ func SetConditionByErrorReason(conditionManager apis.ConditionManager, condition
reason = ReasonForError(err)
}
message := err.Error()
if old == nil || old.IsTrue() || old.GetMessage() != message || old.GetReason() != reason {
if old == nil || !old.IsFalse() || old.GetMessage() != message || old.GetReason() != reason {
conditionManager.MarkFalse(condition, reason, message)
}
} else {
if old == nil || old.IsFalse() {
if old == nil || !old.IsTrue() {
conditionManager.MarkTrue(condition)
}
}
Expand Down
40 changes: 40 additions & 0 deletions apis/meta/v1alpha1/condition_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -362,6 +362,46 @@ func TestSetConditonByError(t *testing.T) {
SetConditionByError(conditionManager, apis.ConditionSucceeded, nil)
})

t.Run("err is nil and from unknown status", func(t *testing.T) {
ctrl := gomock.NewController(t)
defer ctrl.Finish()

conditionManager := apismock.NewMockConditionManager(ctrl)

conditionManager.EXPECT().GetCondition(apis.ConditionSucceeded).Return(&apis.Condition{
Type: apis.ConditionSucceeded,
Status: corev1.ConditionUnknown,
})

conditionManager.EXPECT().
MarkTrue(apis.ConditionSucceeded).
Times(1)

SetConditionByError(conditionManager, apis.ConditionSucceeded, nil)
})

t.Run("err is not nil and from unknown status", func(t *testing.T) {
ctrl := gomock.NewController(t)
defer ctrl.Finish()

conditionManager := apismock.NewMockConditionManager(ctrl)

err := errors.NewBadRequest("some reason")

conditionManager.EXPECT().GetCondition(apis.ConditionSucceeded).Return(&apis.Condition{
Type: apis.ConditionSucceeded,
Status: corev1.ConditionUnknown,
Message: err.Error(),
Reason: string(metav1.StatusReasonBadRequest),
})

conditionManager.EXPECT().
MarkFalse(apis.ConditionSucceeded, string(metav1.StatusReasonBadRequest), err.Error()).
Times(1)

SetConditionByError(conditionManager, apis.ConditionSucceeded, err)
})

t.Run("Success condition not changed", func(t *testing.T) {
ctrl := gomock.NewController(t)
defer ctrl.Finish()
Expand Down
2 changes: 1 addition & 1 deletion apis/meta/v1alpha1/createdBy_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import (
rbacv1 "k8s.io/api/rbac/v1"
)

// Stores a list of created information.
// CreatedBy stores a list of created information.
type CreatedBy struct {
// Reference to the user that created the object. Any Kubernetes `Subject` is accepted.
// +optional
Expand Down
2 changes: 1 addition & 1 deletion apis/meta/v1alpha1/createmeta_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ limitations under the License.

package v1alpha1

// CreateBranchParams params for create file in server
// CreateRepoFileParams params for create file in server
type CreateRepoFileParams struct {
// Branch target branch to create file
Branch string `json:"branch"`
Expand Down
3 changes: 3 additions & 0 deletions apis/meta/v1alpha1/image_config_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,13 +24,16 @@ import (
var ImageConfigGVK = GroupVersion.WithKind("ImageConfig")

// ImageConfig object for plugins
// +k8s:deepcopy-gen=false
type ImageConfig struct {
metav1.TypeMeta `json:",inline"`
metav1.ObjectMeta `json:"metadata,omitempty"`

Spec ImageConfigSpec `json:"spec"`
}

// +k8s:deepcopy-gen=false

type ImageConfigSpec struct {
Config v1.ImageConfig `json:"config"`
}
4 changes: 2 additions & 2 deletions apis/meta/v1alpha1/listmeta_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ func (opt ListOptions) GetSearchFirstElement(key string) (value string) {
return
}

// When ListOption page less than zero, set default value.
// DefaultPager when ListOption page less than zero, set default value.
func (opt *ListOptions) DefaultPager() {
if opt.ItemsPerPage < 1 {
opt.ItemsPerPage = common.DefaultPerPage
Expand Down Expand Up @@ -129,7 +129,7 @@ type ArtifactOptions struct {
Artifact string `json:"artifact"`
}

// IssueOption path params
// IssueOptions path params
type IssueOptions struct {
// Project identity name
Identity string `json:"identity"`
Expand Down
2 changes: 1 addition & 1 deletion apis/meta/v1alpha1/objectcondition_set.go
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ func (o *ObjectConditionSet) GetObjectConditions() ObjectConditions {
return o.accessor.GetObjectConditions()
}

// GetObjectConditions set conditions
// SetObjectConditions set conditions
func (o *ObjectConditionSet) SetObjectConditions(objcs ObjectConditions) {
o.accessor.SetObjectConditions(objcs)
}
Expand Down
3 changes: 2 additions & 1 deletion apis/meta/v1alpha1/objectreference.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ import (
"k8s.io/apimachinery/pkg/types"
)

// IsTheSameObjRef compares two corev1.ObjectReference comparing:
// IsTheSameObject compares two corev1.ObjectReference comparing:
// APIVersion, Kind, Name and Namespace. All other attributes are ignored
func IsTheSameObject(obj, compared corev1.ObjectReference) bool {
return obj.APIVersion == compared.APIVersion &&
Expand Down Expand Up @@ -58,6 +58,7 @@ func GetNamespacedNameFromRef(ref *corev1.ObjectReference) (named types.Namespac
return
}

// ObjectRefOptionsFunc is a function that can be used to modify an object reference
// +k8s:deepcopy-gen=false
type ObjectRefOptionsFunc func(obj metav1.Object, ref *corev1.ObjectReference)

Expand Down
2 changes: 1 addition & 1 deletion apis/meta/v1alpha1/triggeredby_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ var DefinitionTriggeredTypeValues = definitionTriggeredTypeValuesType{
Automated: "Automated",
}

// Stores a list of triggered information such as: Entity that triggered,
// TriggeredBy stores a list of triggered information such as: Entity that triggered,
// reference of an object that could have triggered, and event that triggered.
type TriggeredBy struct {
// Reference to the user that triggered the object. Any Kubernetes `Subject` is accepted.
Expand Down
Loading