Skip to content

Commit

Permalink
Lint fixes; Better DevOps linter (#533)
Browse files Browse the repository at this point in the history
  • Loading branch information
draychev authored and akshaysngupta committed Sep 12, 2019
1 parent e28a138 commit e23b8ee
Show file tree
Hide file tree
Showing 25 changed files with 108 additions and 42 deletions.
3 changes: 1 addition & 2 deletions .pipelines/build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,7 @@ steps:

- script: |
go get -u golang.org/x/lint/golint
golint $(find . -type f -name '*.go' -not -path "./vendor/*") > /tmp/lint.out
cat /tmp/lint.out
golint $(go list ./... | grep -v /vendor/) | tee /tmp/lint.out
if [ -s /tmp/lint.out ]; then
echo -e "\e[101;97m golint FAILED \e[0m"``
exit 1
Expand Down
12 changes: 0 additions & 12 deletions cmd/appgw-ingress/errors.go

This file was deleted.

14 changes: 7 additions & 7 deletions cmd/appgw-ingress/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,16 +48,16 @@ import (
const (
verbosityFlag = "verbosity"
maxAuthRetryCount = 10
tenSeconds = 10 * time.Second
thirtySeconds = 30 * time.Second
retryPause = 10 * time.Second
resyncPause = 30 * time.Second
)

var (
flags = pflag.NewFlagSet(`appgw-ingress`, pflag.ExitOnError)
inCluster = flags.Bool("in-cluster", true, "If running in a Kubernetes cluster, use the pod secrets for creating a Kubernetes client. Optional.")
apiServerHost = flags.String("apiserver-host", "", "The address of the Kubernetes API Server. Optional if running in cluster; if omitted, local discovery is attempted.")
kubeConfigFile = flags.String("kubeconfig", "", "Path to kubeconfig file with authorization and master location information.")
resyncPeriod = flags.Duration("sync-period", thirtySeconds, "Interval at which to re-list and confirm cloud resources.")
resyncPeriod = flags.Duration("sync-period", resyncPause, "Interval at which to re-list and confirm cloud resources.")
versionInfo = flags.Bool("version", false, "Print version")
verbosity = flags.Int(verbosityFlag, 1, "Set logging verbosity level")
)
Expand Down Expand Up @@ -210,8 +210,8 @@ func getAuthorizerWithRetry(env environment.EnvVariables, maxAuthRetryCount int)
return nil, errors.New("failed obtaining auth token")
}
retryCount++
glog.Errorf("Failed fetching authorization token for ARM. Will retry in %v. Error: %s", tenSeconds, err)
time.Sleep(tenSeconds)
glog.Errorf("Failed fetching authorization token for ARM. Will retry in %v. Error: %s", retryPause, err)
time.Sleep(retryPause)
}
}

Expand Down Expand Up @@ -241,8 +241,8 @@ func waitForAzureAuth(env environment.EnvVariables, appGwClient n.ApplicationGat
return errors.New("failed arm auth")
}
retryCount++
glog.Errorf("Failed fetching config for App Gateway instance %s. Will retry in %v. Error: %s", env.AppGwName, tenSeconds, err)
time.Sleep(tenSeconds)
glog.Errorf("Failed fetching config for App Gateway instance %s. Will retry in %v. Error: %s", env.AppGwName, retryPause, err)
time.Sleep(retryPause)
}
}

Expand Down
26 changes: 25 additions & 1 deletion pkg/appgw/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,18 +8,42 @@ package appgw
import "errors"

var (
// ErrEmptyConfig is an error.
ErrEmptyConfig = errors.New("empty App Gateway config")
ErrResolvingBackendPortForService = errors.New("unable to resolve backend port for some services")

// ErrMultipleServiceBackendPortBinding is an error.
ErrMultipleServiceBackendPortBinding = errors.New("more than one service-backend port binding is not allowed")

// ErrGeneratingProbes is an error.
ErrGeneratingProbes = errors.New("unable to generate health probes")

// ErrGeneratingBackendSettings is an error.
ErrGeneratingBackendSettings = errors.New("unable to generate backend http settings")

// ErrGeneratingPools is an error.
ErrGeneratingPools = errors.New("unable to generate backend address pools")

// ErrGeneratingListeners is an error.
ErrGeneratingListeners = errors.New("unable to generate frontend listeners")

// ErrGeneratingRoutingRules is an error.
ErrGeneratingRoutingRules = errors.New("unable to generate request routing rules")

// ErrKeyNoDefaults is an error.
ErrKeyNoDefaults = errors.New("either a DefaultRedirectConfiguration or (DefaultBackendAddressPool + DefaultBackendHTTPSettings) must be configured")

// ErrKeyEitherDefaults is an error.
ErrKeyEitherDefaults = errors.New("URL Path Map must have either DefaultRedirectConfiguration or (DefaultBackendAddressPool + DefaultBackendHTTPSettings) but not both")

// ErrKeyNoBorR is an error.
ErrKeyNoBorR = errors.New("A valid path rule must have one of RedirectConfiguration or (BackendAddressPool + BackendHTTPSettings)")

// ErrKeyEitherBorR is an error.
ErrKeyEitherBorR = errors.New("A Path Rule must have either RedirectConfiguration or (BackendAddressPool + BackendHTTPSettings) but not both")

// ErrKeyNoPrivateIP is an error.
ErrKeyNoPrivateIP = errors.New("A Private IP must be present in the Application Gateway FrontendIPConfiguration if the controller is configured to UsePrivateIP for routing rules")

// ErrKeyNoPublicIP is an error.
ErrKeyNoPublicIP = errors.New("A Public IP must be present in the Application Gateway FrontendIPConfiguration")
)
6 changes: 3 additions & 3 deletions pkg/appgw/frontend_listeners.go
Original file line number Diff line number Diff line change
Expand Up @@ -169,15 +169,15 @@ func (c *appGwConfigBuilder) newListener(cbCtx *ConfigBuilderContext, listenerID

func (c *appGwConfigBuilder) groupListenersByListenerIdentifier(cbCtx *ConfigBuilderContext) map[listenerIdentifier]*n.ApplicationGatewayHTTPListener {
listeners, ports := c.getListeners(cbCtx)
portsById := make(map[string]n.ApplicationGatewayFrontendPort)
portsByID := make(map[string]n.ApplicationGatewayFrontendPort)
for _, port := range *ports {
portsById[*port.ID] = port
portsByID[*port.ID] = port
}

listenersByID := make(map[listenerIdentifier]*n.ApplicationGatewayHTTPListener)
// Update the listenerMap with the final listener lists
for idx, listener := range *listeners {
port := portsById[*listener.FrontendPort.ID]
port := portsByID[*listener.FrontendPort.ID]
listenerID := listenerIdentifier{
HostName: *listener.HostName,
FrontendPort: Port(*port.Port),
Expand Down
2 changes: 2 additions & 0 deletions pkg/appgw/identifier.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ func (agw Identifier) gatewayResourceID(subResourceKind string, resourceName str
return agw.resourceID("Microsoft.Network", "applicationGateways", resourcePath)
}

// AddressPoolID generates an ID for a backend address pool.
func (agw Identifier) AddressPoolID(poolName string) string {
return agw.gatewayResourceID("backendAddressPools", poolName)
}
Expand All @@ -45,6 +46,7 @@ func (agw Identifier) sslCertificateID(certname string) string {
return agw.gatewayResourceID("sslCertificates", certname)
}

// HTTPSettingsID generates an ID for App Gateway HTTP settings resource.
func (agw Identifier) HTTPSettingsID(settingsName string) string {
return agw.gatewayResourceID("backendHttpSettingsCollection", settingsName)
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/appgw/ingress_rules.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ func (c *appGwConfigBuilder) getFrontendPortsFromIngress(ingress *v1beta1.Ingres
}

ruleFrontendPorts, _ := c.processIngressRule(rule, ingress, env)
for port, _ := range ruleFrontendPorts {
for port := range ruleFrontendPorts {
frontendPorts[port] = nil
}
}
Expand Down
3 changes: 3 additions & 0 deletions pkg/appgw/internaltypes.go
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,10 @@ func generatePathRuleName(namespace, ingress, suffix string) string {
return formatPropName(fmt.Sprintf("%s%s-%s-%s-%s", agPrefix, prefixPathRule, namespace, ingress, suffix))
}

// DefaultBackendHTTPSettingsName is the name to be assigned to App Gateway's default HTTP settings resource.
var DefaultBackendHTTPSettingsName = fmt.Sprintf("%sdefaulthttpsetting", agPrefix)

// DefaultBackendAddressPoolName is the name to be assigned to App Gateway's default backend pool resource.
var DefaultBackendAddressPoolName = fmt.Sprintf("%sdefaultaddresspool", agPrefix)

func defaultProbeName(protocol n.ApplicationGatewayProtocol) string {
Expand Down
1 change: 1 addition & 0 deletions pkg/appgw/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,4 +38,5 @@ func (cbCtx *ConfigBuilderContext) InIngressList(ingress *v1beta1.Ingress) bool
return false
}

// Port is a type alias for int32, representing a port number.
type Port int32
1 change: 1 addition & 0 deletions pkg/brownfield/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,5 +8,6 @@ package brownfield
import "errors"

var (
// ErrListenerLookup is an error.
ErrListenerLookup = errors.New("failed looking up listener")
)
17 changes: 9 additions & 8 deletions pkg/brownfield/targets.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import (
// TargetBlacklist is a list of Targets, which AGIC is not allowed to apply configuration for.
type TargetBlacklist *[]Target

// TargetPath is a string type alias.
type TargetPath string

// Target uniquely identifies a subset of App Gateway configuration, which AGIC will manage or be prohibited from managing.
Expand Down Expand Up @@ -73,28 +74,28 @@ func (p TargetPath) lower() string {
return strings.ToLower(string(p))
}

func (thisPath TargetPath) contains(otherPath TargetPath) bool {
if thisPath == "" || thisPath == "*" || thisPath == "/*" {
func (p TargetPath) contains(otherPath TargetPath) bool {
if p == "" || p == "*" || p == "/*" {
return true
}

// For strings that do not end with a * - do exact match
if !strings.HasSuffix(thisPath.lower(), "*") {
return thisPath.lower() == otherPath.lower()
if !strings.HasSuffix(p.lower(), "*") {
return p.lower() == otherPath.lower()
}

// "/x/*" contains "/x"
if strings.TrimRight(thisPath.lower(), "/*") == strings.TrimRight(otherPath.lower(), "/*") {
if strings.TrimRight(p.lower(), "/*") == strings.TrimRight(otherPath.lower(), "/*") {
return true
}

if len(thisPath) > len(otherPath) {
if len(p) > len(otherPath) {
return false
}

thisPathChunks := strings.Split(thisPath.lower(), "/")
thisPathChunks := strings.Split(p.lower(), "/")
otherPathChunks := strings.Split(otherPath.lower(), "/")
for idx, _ := range thisPathChunks {
for idx := range thisPathChunks {
if thisPathChunks[idx] == "*" {
return true
}
Expand Down
2 changes: 2 additions & 0 deletions pkg/brownfield/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,9 @@ import (
n "github.com/Azure/azure-sdk-for-go/services/network/mgmt/2019-06-01/network"
)

// Logger is an abstraction over a logging facility.
type Logger interface {
// Info is the a function allowing us to log messages.
Info(args ...interface{})
}

Expand Down
5 changes: 4 additions & 1 deletion pkg/controller/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,9 @@ package controller
import "errors"

var (
ErrFetchingAppGatewayConfig = errors.New("unable to get specified AppGateway")
// ErrFetchingAppGatewayConfig is an error.
ErrFetchingAppGatewayConfig = errors.New("unable to get specified AppGateway")

// ErrDeployingAppGatewayConfig is an error.
ErrDeployingAppGatewayConfig = errors.New("unable to deploy App Gateway config")
)
6 changes: 4 additions & 2 deletions pkg/health/health.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,11 @@ package health

import "net/http"

// Probe is a type alias for a function.
type Probe func() bool

type HealthProbes interface {
// Probes is the interface for liveness and readiness probes
type Probes interface {
Liveness() bool
Readiness() bool
}
Expand All @@ -24,7 +26,7 @@ func makeHandler(router *http.ServeMux, url string, probe Probe) {
}

// NewHealthMux makes a new *http.ServeMux
func NewHealthMux(healthProbes HealthProbes) *http.ServeMux {
func NewHealthMux(healthProbes Probes) *http.ServeMux {
router := http.NewServeMux()
var handlers = map[string]Probe{
"/health/ready": healthProbes.Readiness,
Expand Down
21 changes: 21 additions & 0 deletions pkg/k8scontext/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,15 +8,36 @@ package k8scontext
import "errors"

var (
// ErrorFetchingEnpdoints is an error.
ErrorFetchingEnpdoints = errors.New("FetchingEndpoints")

// ErrorUnknownSecretType is an error.
ErrorUnknownSecretType = errors.New("unknown secret type")

// ErrorCreatingFile is an error.
ErrorCreatingFile = errors.New("unable to create temp file")

// ErrorMalformedSecret is an error.
ErrorMalformedSecret = errors.New("malformed secret")

// ErrorWritingToFile is an error.
ErrorWritingToFile = errors.New("unable to write to file")

// ErrorExportingWithOpenSSL is an error.
ErrorExportingWithOpenSSL = errors.New("failed export with OpenSSL")

// ErrorInformersNotInitialized is an error.
ErrorInformersNotInitialized = errors.New("informers are not initialized")

// ErrorFailedInitialCacheSync is an error.
ErrorFailedInitialCacheSync = errors.New("failed initial sync of resources required for ingress")

// ErrorNoNodesFound is an error.
ErrorNoNodesFound = errors.New("no nodes were found in the node list")

// ErrorUnrecognizedNodeProviderPrefix is an error.
ErrorUnrecognizedNodeProviderPrefix = errors.New("providerID is not prefixed with azure://")

// ErrorUnableToUpdateIngress is an error.
ErrorUnableToUpdateIngress = errors.New("ingress status update")
)
1 change: 1 addition & 0 deletions pkg/k8scontext/secretstore.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ func (s *SecretsStore) delete(secretKey string) {
s.Cache.Delete(secretKey)
}

// ConvertSecret converts a secret to a PKCS12.
func (s *SecretsStore) ConvertSecret(secretKey string, secret *v1.Secret) error {
s.conversionSync.Lock()
defer s.conversionSync.Unlock()
Expand Down
1 change: 1 addition & 0 deletions pkg/tests/fixtures/app_gateway.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
"github.com/Azure/go-autorest/autorest/to"
)

// GetAppGateway creates an ApplicationGateway struct.
func GetAppGateway() n.ApplicationGateway {
// The order of the lists below is important as we reference these by index in unit tests.
return n.ApplicationGateway{
Expand Down
3 changes: 3 additions & 0 deletions pkg/tests/fixtures/certificates.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,18 +21,21 @@ const (
CertificateName3 = "Certificate-3"
)

// GetCertificate1 generates a certificate.
func GetCertificate1() n.ApplicationGatewaySslCertificate {
return n.ApplicationGatewaySslCertificate{
Name: to.StringPtr(CertificateName1),
}
}

// GetCertificate2 generates a certificate.
func GetCertificate2() n.ApplicationGatewaySslCertificate {
return n.ApplicationGatewaySslCertificate{
Name: to.StringPtr(CertificateName2),
}
}

// GetCertificate3 generates a certificate.
func GetCertificate3() n.ApplicationGatewaySslCertificate {
return n.ApplicationGatewaySslCertificate{
Name: to.StringPtr(CertificateName3),
Expand Down
4 changes: 4 additions & 0 deletions pkg/tests/fixtures/http_settings.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
)

const (
// DefaultBackendHTTPSettingsName is a string constant.
DefaultBackendHTTPSettingsName = "DefaultBackendHTTPSettings"

// BackendHTTPSettingsName1 is a string constant.
Expand All @@ -18,6 +19,7 @@ const (
BackendHTTPSettingsName3 = "BackendHTTPSettings-3"
)

// GetHTTPSettings1 generates HTTP settings.
func GetHTTPSettings1() n.ApplicationGatewayBackendHTTPSettings {
return n.ApplicationGatewayBackendHTTPSettings{
ApplicationGatewayBackendHTTPSettingsPropertiesFormat: &n.ApplicationGatewayBackendHTTPSettingsPropertiesFormat{
Expand All @@ -27,6 +29,7 @@ func GetHTTPSettings1() n.ApplicationGatewayBackendHTTPSettings {
}
}

// GetHTTPSettings2 generates HTTP settings.
func GetHTTPSettings2() n.ApplicationGatewayBackendHTTPSettings {
return n.ApplicationGatewayBackendHTTPSettings{
ApplicationGatewayBackendHTTPSettingsPropertiesFormat: &n.ApplicationGatewayBackendHTTPSettingsPropertiesFormat{
Expand All @@ -36,6 +39,7 @@ func GetHTTPSettings2() n.ApplicationGatewayBackendHTTPSettings {
}
}

// GetHTTPSettings3 generates HTTP settings.
func GetHTTPSettings3() n.ApplicationGatewayBackendHTTPSettings {
return n.ApplicationGatewayBackendHTTPSettings{
ApplicationGatewayBackendHTTPSettingsPropertiesFormat: &n.ApplicationGatewayBackendHTTPSettingsPropertiesFormat{
Expand Down
1 change: 1 addition & 0 deletions pkg/tests/fixtures/ingress.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"github.com/Azure/application-gateway-kubernetes-ingress/pkg/tests"
)

// GetIngress creates an Ingress struct.
func GetIngress() *v1beta1.Ingress {
return &v1beta1.Ingress{
Spec: v1beta1.IngressSpec{
Expand Down
1 change: 1 addition & 0 deletions pkg/tests/fixtures/ips.go
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package fixtures

const (
// DefaultIPName is a string constant.
DefaultIPName = "appGatewayFrontendIP"
)
Loading

0 comments on commit e23b8ee

Please sign in to comment.