Skip to content

Commit

Permalink
linter: Add golangci-lint configuration and fix all lint warnings
Browse files Browse the repository at this point in the history
  • Loading branch information
vkareh committed Jun 2, 2020
1 parent 1c2e43b commit 6223e8f
Show file tree
Hide file tree
Showing 24 changed files with 191 additions and 147 deletions.
39 changes: 39 additions & 0 deletions .golangci.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
#
# Copyright (c) 2020 Red Hat, Inc.
#
# 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.
#

run:
skip-dirs: (assets|docs|templates)
issues-exit-code: 1
linters:
disable-all: true
enable:
- deadcode
- gas
- goconst
- gofmt
- golint
- govet
- ineffassign
- interfacer
- lll
- maligned
- megacheck
- misspell
- structcheck
- unconvert
- unparam
- varcheck
- whitespace
35 changes: 16 additions & 19 deletions cmd/create/cluster/cmd.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,23 +40,23 @@ import (

var args struct {
// Basic options
private bool
multiAZ bool
expirationDuration time.Duration
expirationTime string
name string
region string
multiAZ bool
version string
expirationTime string
expirationDuration time.Duration

// Scaling options
computeMachineType string
computeNodes int

// Networking options
hostPrefix int
machineCIDR net.IPNet
serviceCIDR net.IPNet
podCIDR net.IPNet
hostPrefix int
private bool
}

var Cmd = &cobra.Command{
Expand Down Expand Up @@ -126,7 +126,8 @@ func init() {
&args.computeNodes,
"compute-nodes",
0,
"Number of worker nodes to provision per zone. Single zone clusters need at least 4 nodes, while multizone clusters need at least 9 nodes (3 per zone) for resiliency.\n",
"Number of worker nodes to provision per zone. Single zone clusters need at least 4 nodes, "+
"while multizone clusters need at least 9 nodes (3 per zone) for resiliency.\n",
)

flags.IPNetVar(
Expand All @@ -151,7 +152,8 @@ func init() {
&args.hostPrefix,
"host-prefix",
0,
"Subnet prefix length to assign to each individual node. For example, if host prefix is set to \"23\", then each node is assigned a /23 subnet out of the given CIDR.",
"Subnet prefix length to assign to each individual node. For example, if host prefix is set "+
"to \"23\", then each node is assigned a /23 subnet out of the given CIDR.",
)
flags.BoolVar(
&args.private,
Expand Down Expand Up @@ -196,10 +198,9 @@ func run(cmd *cobra.Command, _ []string) {
if region == "" {
region, err = interactive.GetInput("AWS region")
if err != nil {
reporter.Errorf("expected a valid AWS region: %v", err)
reporter.Errorf("Expected a valid AWS region: %v", err)
os.Exit(1)
}

}

// Create the client for the OCM API:
Expand Down Expand Up @@ -240,7 +241,7 @@ func run(cmd *cobra.Command, _ []string) {
private = &args.private
}

clusterConfig := clusterprovider.ClusterSpec{
clusterConfig := clusterprovider.Spec{
Name: name,
Region: region,
MultiAZ: args.multiAZ,
Expand All @@ -267,15 +268,15 @@ func run(cmd *cobra.Command, _ []string) {
clusterID := cluster.ID()
clusterName := cluster.Name()
reporter.Infof("Creating cluster with identifier '%s' and name '%s'", clusterID, clusterName)
reporter.Infof("To view list of clusters and their status, run `moactl list clusters`")
reporter.Infof("To view list of clusters and their status, run 'moactl list clusters'")

reporter.Infof("Cluster '%s' has been created.", clusterName)
reporter.Infof(
"Once the cluster is 'Ready' you will need to add an Identity Provider " +
"and define the list of cluster administrators. See `moactl create idp --help` " +
"and `moactl create user --help` for more information.")
"and define the list of cluster administrators. See 'moactl create idp --help' " +
"and 'moactl create user --help' for more information.")
reporter.Infof(
"To determine when your cluster is Ready, run `moactl describe cluster %s`.",
"To determine when your cluster is Ready, run 'moactl describe cluster %s'.",
clusterName,
)
}
Expand Down Expand Up @@ -311,7 +312,7 @@ func validateVersion(client *cmv1.Client) (version string, err error) {
func validateExpiration() (expiration time.Time, err error) {
// Validate options
if len(args.expirationTime) > 0 && args.expirationDuration != 0 {
err = errors.New("At most one of `expiration-time` or `expiration` may be specified")
err = errors.New("At most one of 'expiration-time' or 'expiration' may be specified")
return
}

Expand Down Expand Up @@ -365,7 +366,3 @@ func parseRFC3339(s string) (time.Time, error) {
}
return time.Parse(time.RFC3339, s)
}

func cidrIsEmpty(cidr net.IPNet) bool {
return cidr.String() == "<nil>"
}
8 changes: 5 additions & 3 deletions cmd/create/idp/cmd.go
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,8 @@ func init() {
&args.githubTeams,
"teams",
"",
"GitHub: Only users that are members of at least one of the listed teams will be allowed to log in. The format is <org>/<team>.\n",
"GitHub: Only users that are members of at least one of the listed teams will be allowed to log in. "+
"The format is <org>/<team>.\n",
)

// Google
Expand Down Expand Up @@ -196,7 +197,8 @@ func init() {
&args.openidIssuerURL,
"issuer-url",
"",
"OpenID: The URL that the OpenID Provider asserts as the Issuer Identifier. It must use the https scheme with no URL query parameters or fragment.",
"OpenID: The URL that the OpenID Provider asserts as the Issuer Identifier. "+
"It must use the https scheme with no URL query parameters or fragment.",
)
flags.StringVar(
&args.openidEmail,
Expand Down Expand Up @@ -361,7 +363,7 @@ func run(_ *cobra.Command, _ []string) {

reporter.Infof(
"Identity Provider '%s' has been created. You need to ensure that there is a list "+
"of cluster administrators defined. See `moactl create user --help` for more "+
"of cluster administrators defined. See 'moactl create user --help' for more "+
"information. To login into the console, open %s and click on %s",
idpName, cluster.Console().URL(), idpName,
)
Expand Down
5 changes: 3 additions & 2 deletions cmd/create/idp/github.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ func buildGithubIdp(cluster *cmv1.Cluster, idpName string) (idpBuilder cmv1.Iden
teamsOrOrgs := ""

if organizations != "" && teams != "" {
return idpBuilder, errors.New("GitHub IDP only allows either organizations or teams, but not both.")
return idpBuilder, errors.New("GitHub IDP only allows either organizations or teams, but not both")
}

isInteractive := clientID == "" || clientSecret == "" || (organizations == "" && teams == "")
Expand All @@ -44,7 +44,8 @@ func buildGithubIdp(cluster *cmv1.Cluster, idpName string) (idpBuilder cmv1.Iden
fmt.Println("To use GitHub as an identity provider, you must first register the application:")

if organizations == "" && teams == "" {
teamsOrOrgs, err = interactive.GetInput("List of GitHub organizations or teams that will have access to this cluster")
teamsOrOrgs, err = interactive.GetInput("List of GitHub organizations or teams " +
"that will have access to this cluster")
if err != nil {
return idpBuilder, errors.New("Expected a GitHub organization or team name")
}
Expand Down
4 changes: 2 additions & 2 deletions cmd/create/idp/google.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,14 +38,14 @@ func buildGoogleIdp(cluster *cmv1.Cluster, idpName string) (idpBuilder cmv1.Iden

if isInteractive {
fmt.Println("To use Google as an identity provider, you must first register the application:")
// instructionsURL := "https://docs.openshift.com/dedicated/4/authentication/identity_providers/configuring-google-identity-provider.html"
instructionsURL := "https://console.developers.google.com/projectcreate"
fmt.Println("* Open the following URL:", instructionsURL)
fmt.Println("* Follow the instructions to register your application")

consoleURL := cluster.Console().URL()
oauthURL := strings.Replace(consoleURL, "console-openshift-console", "oauth-openshift", 1)
fmt.Println("* When creating the OAuth client ID, use the following URL for the Authorized redirect URI:", oauthURL+"/oauth2callback/"+idpName)
fmt.Println("* When creating the OAuth client ID, use the following URL for the Authorized redirect URI: ",
oauthURL+"/oauth2callback/"+idpName)

if clientID == "" {
clientID, err = interactive.GetInput("Copy the Client ID provided by Google")
Expand Down
7 changes: 4 additions & 3 deletions cmd/create/idp/ldap.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,15 +27,16 @@ import (
"github.com/openshift/moactl/pkg/interactive"
)

func buildLdapIdp(cluster *cmv1.Cluster, idpName string) (idpBuilder cmv1.IdentityProviderBuilder, err error) {
func buildLdapIdp(_ *cmv1.Cluster, idpName string) (idpBuilder cmv1.IdentityProviderBuilder, err error) {
ldapURL := args.ldapURL
ldapIDs := args.ldapIDs

isInteractive := ldapURL == "" || ldapIDs == ""

if isInteractive {
fmt.Println("To use LDAP as an identity provider, you must first register the application:")
instructionsURL := "https://docs.openshift.com/dedicated/4/authentication/identity_providers/configuring-ldap-identity-provider.html"
instructionsURL := "https://docs.openshift.com/dedicated/4/authentication/" +
"identity_providers/configuring-ldap-identity-provider.html"
fmt.Println("* Open the following URL:", instructionsURL)
fmt.Println("* Follow the instructions to register your application")

Expand All @@ -59,7 +60,7 @@ func buildLdapIdp(cluster *cmv1.Cluster, idpName string) (idpBuilder cmv1.Identi
return idpBuilder, fmt.Errorf("Expected a valid LDAP URL: %v", err)
}
if parsedLdapURL.Scheme != "ldap" && parsedLdapURL.Scheme != "ldaps" {
return idpBuilder, errors.New("Expected LDAP URL to have an ldap:// or ldaps:// scheme.")
return idpBuilder, errors.New("Expected LDAP URL to have an ldap:// or ldaps:// scheme")
}

// Create LDAP attributes
Expand Down
21 changes: 12 additions & 9 deletions cmd/create/idp/openid.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,17 +35,20 @@ func buildOpenidIdp(cluster *cmv1.Cluster, idpName string) (idpBuilder cmv1.Iden
name := args.openidName
username := args.openidUsername

isInteractive := clientID == "" || clientSecret == "" || issuerURL == "" || (email == "" && name == "" && username == "")
isInteractive := clientID == "" || clientSecret == "" || issuerURL == "" ||
(email == "" && name == "" && username == "")

if isInteractive {
fmt.Println("To use OpenID as an identity provider, you must first register the application:")
instructionsURL := "https://docs.openshift.com/dedicated/4/authentication/identity_providers/configuring-oidc-identity-provider.html"
instructionsURL := "https://docs.openshift.com/dedicated/4/authentication/" +
"identity_providers/configuring-oidc-identity-provider.html"
fmt.Println("* Open the following URL:", instructionsURL)
fmt.Println("* Follow the instructions to register your application")

consoleURL := cluster.Console().URL()
oauthURL := strings.Replace(consoleURL, "console-openshift-console", "oauth-openshift", 1)
fmt.Println("* When creating the OpenID, use the following URL for the Authorized redirect URI:", oauthURL+"/oauth2callback/"+idpName)
fmt.Println("* When creating the OpenID, use the following URL for the Authorized redirect URI: ",
oauthURL+"/oauth2callback/"+idpName)

if clientID == "" {
clientID, err = interactive.GetInput("Copy the Client ID provided by the OpenID Provider")
Expand All @@ -71,21 +74,21 @@ func buildOpenidIdp(cluster *cmv1.Cluster, idpName string) (idpBuilder cmv1.Iden
if email == "" {
email, err = interactive.GetInput("Claim mappings to use as the email address")
if err != nil {
return idpBuilder, errors.New("Expected a list of claims to use as the email address.")
return idpBuilder, errors.New("Expected a list of claims to use as the email address")
}
}

if name == "" {
name, err = interactive.GetInput("Claim mappings to use as the display name")
if err != nil {
return idpBuilder, errors.New("Expected a list of claims to use as the display name.")
return idpBuilder, errors.New("Expected a list of claims to use as the display name")
}
}

if username == "" {
username, err = interactive.GetInput("Claim mappings to use as the preferred username")
if err != nil {
return idpBuilder, errors.New("Expected a list of claims to use as the preferred username.")
return idpBuilder, errors.New("Expected a list of claims to use as the preferred username")
}
}
}
Expand All @@ -99,13 +102,13 @@ func buildOpenidIdp(cluster *cmv1.Cluster, idpName string) (idpBuilder cmv1.Iden
return idpBuilder, fmt.Errorf("Expected a valid OpenID issuer URL: %v", err)
}
if parsedIssuerURL.Scheme != "https" {
return idpBuilder, errors.New("Expected OpenID issuer URL to use an https:// scheme.")
return idpBuilder, errors.New("Expected OpenID issuer URL to use an https:// scheme")
}
if parsedIssuerURL.RawQuery != "" {
return idpBuilder, errors.New("OpenID issuer URL must not have query parameters.")
return idpBuilder, errors.New("OpenID issuer URL must not have query parameters")
}
if parsedIssuerURL.Fragment != "" {
return idpBuilder, errors.New("OpenID issuer URL must not have a fragment.")
return idpBuilder, errors.New("OpenID issuer URL must not have a fragment")
}

// Build OpenID Claims
Expand Down
3 changes: 2 additions & 1 deletion cmd/create/ingress/cmd.go
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,8 @@ func init() {
&args.labelMatch,
"label-match",
"",
"Label match for ingress. Format should be a comma-separated list of 'key=value'. If no label is specified, all routes will be exposed on both routers.",
"Label match for ingress. Format should be a comma-separated list of 'key=value'. "+
"If no label is specified, all routes will be exposed on both routers.",
)
}

Expand Down
7 changes: 4 additions & 3 deletions cmd/edit/cluster/cmd.go
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,8 @@ func init() {
&args.computeNodes,
"compute-nodes",
0,
"Number of worker nodes to provision per zone. Single zone clusters need at least 4 nodes, while multizone clusters need at least 9 nodes (3 per zone) for resiliency.",
"Number of worker nodes to provision per zone. Single zone clusters need at least 4 nodes, "+
"while multizone clusters need at least 9 nodes (3 per zone) for resiliency.",
)

// Networking options
Expand Down Expand Up @@ -200,7 +201,7 @@ func run(cmd *cobra.Command, argv []string) {
clusterAdmins = &args.clusterAdmins
}

clusterConfig := clusterprovider.ClusterSpec{
clusterConfig := clusterprovider.Spec{
Expiration: expiration,
ComputeNodes: args.computeNodes,
Private: private,
Expand All @@ -221,7 +222,7 @@ func run(cmd *cobra.Command, argv []string) {
func validateExpiration() (expiration time.Time, err error) {
// Validate options
if len(args.expirationTime) > 0 && args.expirationDuration != 0 {
err = errors.New("At most one of `expiration-time` or `expiration` may be specified")
err = errors.New("At most one of 'expiration-time' or 'expiration' may be specified")
return
}

Expand Down
5 changes: 3 additions & 2 deletions cmd/edit/ingress/cmd.go
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,8 @@ func init() {
&args.labelMatch,
"label-match",
"",
"Label match for ingress. Format should be a comma-separated list of 'key=value'. If no label is specified, all routes will be exposed on both routers.",
"Label match for ingress. Format should be a comma-separated list of 'key=value'. "+
"If no label is specified, all routes will be exposed on both routers.",
)
}

Expand Down Expand Up @@ -190,7 +191,7 @@ func run(cmd *cobra.Command, argv []string) {

// Edit API endpoint instead of ingresses
if ingressID == "api" {
clusterConfig := clusterprovider.ClusterSpec{
clusterConfig := clusterprovider.Spec{
Private: private,
}

Expand Down
10 changes: 7 additions & 3 deletions cmd/initialize/cmd.go
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,10 @@ func run(cmd *cobra.Command, argv []string) {
if cfg != nil {
// Check that credentials in the config file are valid
isLoggedIn, err = cfg.Armed()
if err != nil {
reporter.Errorf("Failed to determine if user is logged in: %v", err)
os.Exit(1)
}
}

if isLoggedIn {
Expand Down Expand Up @@ -190,12 +194,12 @@ func run(cmd *cobra.Command, argv []string) {
os.Exit(1)
}

reporter.Infof("Admin user '%s' deleted successfuly!", aws.AdminUserName)
reporter.Infof("Admin user '%s' deleted successfully!", aws.AdminUserName)
os.Exit(0)
}

// Validate AWS SCP/IAM Permissions
// Call `verify permisisons` as partof init
// Call `verify permissions` as part of init
permissions.Cmd.Run(cmd, argv)

// Validate AWS quota
Expand All @@ -210,7 +214,7 @@ func run(cmd *cobra.Command, argv []string) {
os.Exit(1)
}
if created {
reporter.Infof("Admin user '%s' created successfuly!", aws.AdminUserName)
reporter.Infof("Admin user '%s' created successfully!", aws.AdminUserName)
} else {
reporter.Infof("Admin user '%s' already exists!", aws.AdminUserName)
}
Expand Down
Loading

0 comments on commit 6223e8f

Please sign in to comment.