Add make verify target

- [x] Add make verify target
- [x] Add verify-lint target
- [x] Add verify-gofmt target
- [x] Add verify-go-vet target
- [x] Fix make verify-imports target to error when verify-imports fails.
- [x] Fix golint errors in codebase
- [x] Remove fmt and vet targets from build targets (necessary for make verify to work)
- [x] Add .PHONY to Makefile targets that don't produce files of the same name.

Author: Thomas Wiest

Makefile

@@ -8,9 +8,10 @@ VERIFY_IMPORTS_CONFIG = build/verify-imports/import-rules.yaml
 # Image URL to use all building/pushing image targets
 IMG ?= controller:latest
 
-all: test build
+all: fmt vet test build
 
 # Run tests
+.PHONY: test
 test: generate fmt vet manifests
 	go test ./pkg/... ./cmd/... -coverprofile cover.out
 
@@ -20,56 +21,93 @@ build: manager hiveutil
 
 
 # Build manager binary
-manager: generate fmt vet
+manager: generate
 	go build -o bin/manager github.com/openshift/hive/cmd/manager
 
 # Build hiveutil binary
-hiveutil: fmt vet
+hiveutil:
 	go build -o bin/hiveutil github.com/openshift/hive/contrib/cmd/hiveutil
 
 # Run against the configured Kubernetes cluster in ~/.kube/config
+.PHONY: run
 run: generate fmt vet
 	go run ./cmd/manager/main.go
 
 # Install CRDs into a cluster
+.PHONY: install
 install: manifests
 	kubectl apply -f config/crds
 
 # Deploy controller in the configured Kubernetes cluster in ~/.kube/config
+.PHONY: deploy
 deploy: manifests
 	kubectl apply -f config/crds
 	kustomize build config/default | kubectl apply -f -
 
 # Generate manifests e.g. CRD, RBAC etc.
+.PHONY: manifests
 manifests:
 	go run vendor/sigs.k8s.io/controller-tools/cmd/controller-gen/main.go all
 
 # Run go fmt against code
+.PHONY: fmt
 fmt:
 	go fmt ./pkg/... ./cmd/... ./contrib/...
 
 # Run go vet against code
+.PHONY: vet
 vet:
 	go vet ./pkg/... ./cmd/... ./contrib/...
 
+# Run verification tests
+.PHONY: verify
+verify: hiveutil verify-imports verify-gofmt verify-lint verify-go-vet
+
 # Check import naming
+.PHONY: verify-imports
 verify-imports:
 	@echo "Verifying import naming"
-	$(foreach file,$(GOFILES), \
-		$(shell $(BINDIR)/hiveutil verify-imports -c $(VERIFY_IMPORTS_CONFIG) $(file))\
-	)
+	@sh -c \
+	  'for file in $(GOFILES) ; do \
+	     $(BINDIR)/hiveutil verify-imports -c $(VERIFY_IMPORTS_CONFIG) $$file || exit 1 ; \
+	   done'
 
+# Check import naming
+.PHONY: verify-lint
+verify-lint:
+	@echo Verifying golint
+	@sh -c \
+	  'for file in $(GOFILES) ; do \
+	     golint --set_exit_status $$file || exit 1 ; \
+	   done'
+
+.PHONY: verify-gofmt
+verify-gofmt:
+	@echo Verifying gofmt
+	@gofmt -l -s $(SRC_DIRS)>.out 2>&1 || true
+	@[ ! -s .out ] || \
+	  (echo && echo "*** Please 'gofmt -s -d' on the following:" && \
+	  cat .out && echo && rm .out && false)
+	@rm .out
+
+.PHONY: verify-go-vet
+verify-go-vet:
+	@echo Verifying go vet
+	@go vet ./cmd/... ./contrib/... $(go list ./pkg/... | grep -v _generated)
 
 # Generate code
+.PHONY: generate
 generate:
 	go generate ./pkg/... ./cmd/...
 
 # Build the docker image
+.PHONY: docker-build
 docker-build: test
 	docker build . -t ${IMG}
 	@echo "updating kustomize image patch file for manager resource"
 	sed -i'' -e 's@image: .*@image: '"${IMG}"'@' ./config/default/manager_image_patch.yaml
 
 # Push the docker image
+.PHONY: docker-push
 docker-push:
 	docker push ${IMG}

contrib/cmd/hiveutil/main.go

@@ -23,15 +23,15 @@ import (
 	log "github.com/sirupsen/logrus"
 	"github.com/spf13/cobra"
 
-	"github.com/openshift/hive/contrib/pkg/aws_tag_deprovision"
+	"github.com/openshift/hive/contrib/pkg/awstagdeprovision"
 	"github.com/openshift/hive/contrib/pkg/verification"
 )
 
 func main() {
 	log.SetOutput(os.Stdout)
 	log.SetLevel(log.DebugLevel)
 
-	cmd := NewCOUtilityCommand()
+	cmd := newCOUtilityCommand()
 
 	err := cmd.Execute()
 	if err != nil {
@@ -40,7 +40,7 @@ func main() {
 	}
 }
 
-func NewCOUtilityCommand() *cobra.Command {
+func newCOUtilityCommand() *cobra.Command {
 	cmd := &cobra.Command{
 		Use:   "hiveutil SUB-COMMAND",
 		Short: "Utilities for hive",
@@ -49,7 +49,7 @@ func NewCOUtilityCommand() *cobra.Command {
 			cmd.Usage()
 		},
 	}
-	cmd.AddCommand(aws_tag_deprovision.NewDeprovisionAWSWithTagsCommand())
+	cmd.AddCommand(awstagdeprovision.NewDeprovisionAWSWithTagsCommand())
 	cmd.AddCommand(verification.NewVerifyImportsCommand())
 
 	return cmd

…s_tag_deprovision/aws_tag_deprovision.go …g/awstagdeprovision/awstagdeprovision.gocontrib/pkg/aws_tag_deprovision/aws_tag_deprovision.go renamed to contrib/pkg/awstagdeprovision/awstagdeprovision.go contrib/pkg/aws_tag_deprovision/aws_tag_deprovision.go renamed to contrib/pkg/awstagdeprovision/awstagdeprovision.go

@@ -14,7 +14,7 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-package aws_tag_deprovision
+package awstagdeprovision
 
 import (
 	"fmt"
@@ -427,9 +427,9 @@ func deleteVPCs(awsSession *session.Session, filters awsFilter, clusterName stri
 			if err != nil {
 				logger.Debugf("error deleting VPC %v: %v", *vpc.VpcId, err)
 				return false, nil
-			} else {
-				logger.WithField("id", *vpc.VpcId).Info("Deleted VPC")
 			}
+
+			logger.WithField("id", *vpc.VpcId).Info("Deleted VPC")
 		}
 
 		return false, nil
@@ -513,9 +513,9 @@ func deleteNetworkIface(iface *string, ec2Client *ec2.EC2, logger log.FieldLogge
 		if err != nil {
 			logger.Debugf("error deleting network iface: %v", err)
 			return err
-		} else {
-			logger.WithField("id", *i.NetworkInterfaceId).Info("Deleted network interface")
 		}
+
+		logger.WithField("id", *i.NetworkInterfaceId).Info("Deleted network interface")
 	}
 
 	return nil
@@ -604,10 +604,10 @@ func deleteRolesFromInstanceProfile(ip *iam.InstanceProfile, iamClient *iam.IAM,
 		if err != nil {
 			logger.Debugf("error deleting policies from role: %v", err)
 			return err
-		} else {
-			logger.Infof("Deleted all policies from role: %v", *role.RoleName)
 		}
 
+		logger.Infof("Deleted all policies from role: %v", *role.RoleName)
+
 		// detach role from instance profile
 		_, err = iamClient.RemoveRoleFromInstanceProfile(&iam.RemoveRoleFromInstanceProfileInput{
 			InstanceProfileName: ip.InstanceProfileName,
@@ -616,10 +616,10 @@ func deleteRolesFromInstanceProfile(ip *iam.InstanceProfile, iamClient *iam.IAM,
 		if err != nil {
 			logger.Debugf("error removing role from instance profile: %v", err)
 			return err
-		} else {
-			logger.Infof("Removed role %v from instance profile %v", *role.RoleName, *ip.InstanceProfileName)
 		}
 
+		logger.Infof("Removed role %v from instance profile %v", *role.RoleName, *ip.InstanceProfileName)
+
 		// now delete the role
 		// need to loop because this is the only time we'll have the name of the role
 		// now that it has been detached from the instance profile
@@ -748,8 +748,8 @@ func tryDeleteRoleProfileByName(roleName string, profileName string, session *se
 func deleteIAMresources(session *session.Session, filter awsFilter, clusterName string, logger log.FieldLogger) (bool, error) {
 	logger.Debugf("Deleting IAM resources")
 	defer logger.Debugf("Exiting deleting IAM resources")
-	installer_type := []string{"master", "worker", "bootstrap"}
-	for _, t := range installer_type {
+	installerType := []string{"master", "worker", "bootstrap"}
+	for _, t := range installerType {
 		// Naming of IAM resources expected from https://github.com/openshift/installer as follows:
 		// $CLUSTER_NAME-master-role     $CLUSTER_NAME-worker-role    $CLUSTER_NAME-bootstrap-role
 		// $CLUSTER_NAME-master-profile  $CLUSTER_NAME-worker-profile $CLUSTER_NAME-bootstrap-profile
@@ -993,9 +993,9 @@ func deleteRoutesFromTable(rt *ec2.RouteTable, ec2Client *ec2.EC2, logger log.Fi
 		if err != nil {
 			logger.Debugf("error deleting route from route table: %v", err)
 			return err
-		} else {
-			logger.Infof("Deleted route %v from route table %v", *route.DestinationCidrBlock, *rt.RouteTableId)
 		}
+
+		logger.Infof("Deleted route %v from route table %v", *route.DestinationCidrBlock, *rt.RouteTableId)
 	}
 	return nil
 }
@@ -1232,9 +1232,9 @@ func deleteEntriesFromSharedR53Zone(zoneID string, sharedZoneID string, r53Clien
 				})
 				if err != nil {
 					return err
-				} else {
-					logger.Infof("Deleted record %v from r53 zone %v", *sharedEntry.Name, sharedZoneID)
 				}
+
+				logger.Infof("Deleted record %v from r53 zone %v", *sharedEntry.Name, sharedZoneID)
 			}
 		}
 	}
@@ -1305,9 +1305,8 @@ func emptyAndDeleteRoute53Zone(zoneID string, r53Client *route53.Route53, logger
 		})
 		if err != nil {
 			return err
-		} else {
-			logger.Infof("Deleted record %v from r53 zone %v", *entry.Name, zoneID)
 		}
+		logger.Infof("Deleted record %v from r53 zone %v", *entry.Name, zoneID)
 	}
 
 	// now delete zone
@@ -1316,10 +1315,10 @@ func emptyAndDeleteRoute53Zone(zoneID string, r53Client *route53.Route53, logger
 	})
 	if err != nil {
 		return err
-	} else {
-		logger.WithField("id", zoneID).Info("Deleted route53 zone")
 	}
 
+	logger.WithField("id", zoneID).Info("Deleted route53 zone")
+
 	return nil
 }
 

pkg/apis/hive/v1alpha1/clusterdeployment_types.go

@@ -30,6 +30,7 @@ type ClusterDeploymentSpec struct {
 	PlatformSecrets PlatformSecrets `json:"platformSecrets"`
 }
 
+// PlatformSecrets defines the secrets to be used by various clouds.
 type PlatformSecrets struct {
 	// +optional
 	AWS *AWSPlatformSecrets `json:"aws,omitempty"`