Tighten the restrictions of the linter

This change causes the linter to be a bit more complainy. The hope is
that this will cut down on some of the more pedantic issues being caught
in code reviews, and thus reduce the overall time a change spends in the
review process.

This change includes various changes to the codebase to bring it up to
the new standards.

Change-Id: I570d304bca5554404354f972d8a2743279a0171b
This commit is contained in:
Ian Howell 2020-01-08 13:54:13 -06:00
parent b43e4e4fea
commit 49027f4151
39 changed files with 170 additions and 201 deletions

View File

@ -1,5 +1,5 @@
# This file contains all available configuration options
# with their default values.
# with their documentation
# options for analysis running
run:
@ -61,39 +61,92 @@ output:
# all available settings of specific linters
linters-settings:
dupl:
# tokens count to trigger issue, 150 by default if not set here
threshold: 150
errcheck:
# report about not checking of errors in type assetions: `a := b.(MyStruct)`;
# default is false: such cases aren't reported by default.
check-type-assertions: false
check-type-assertions: true
# report about assignment of errors to blank identifier: `num, _ := strconv.Atoi(numStr)`;
# default is false: such cases aren't reported by default.
check-blank: false
check-blank: true
# path to a file containing a list of functions to exclude from checking
# see https://github.com/kisielk/errcheck#excluding-functions for details
# exclude: /path/to/file.txt
govet:
# report about shadowed variables
check-shadowing: true
goconst:
# minimal length of string constant, 3 by default
min-len: 4
# minimal occurrences count to trigger, 3 by default
min-occurrences: 5
gocritic:
# Which checks should be enabled; can't be combined with 'disabled-checks';
# See https://go-critic.github.io/overview#checks-overview
# To check which checks are enabled run `GL_DEBUG=gocritic golangci-lint run`
# By default list of stable checks is used.
enabled-checks:
- appendAssign
- appendCombine
- assignOp
- captLocal
- caseOrder
- commentedOutCode
- commentedOutImport
- defaultCaseOrder
- dupArg
- dupBranchBody
- dupCase
- dupSubExpr
- elseif
- equalFold
- flagDeref
- ifElseChain
- regexpMust
- singleCaseSwitch
- sloppyLen
- switchTrue
- typeAssertChain
- typeSwitchVar
- underef
- unlambda
- unslice
# Which checks should be disabled; can't be combined with 'enabled-checks'; default is empty
# disabled-checks:
# - regexpMust
# Enable multiple checks by tags, run `GL_DEBUG=gocritic golangci-lint run` to see all tags and checks.
# Empty list by default. See https://github.com/go-critic/go-critic#usage -> section "Tags".
# enabled-tags:
# - performance
settings: # settings passed to gocritic
captLocal:
paramsOnly: false
gocyclo:
# minimal code complexity to report, 30 by default (but we recommend 10-20)
min-complexity: 15
gofmt:
# simplify code: gofmt with `-s` option, true by default
simplify: true
goimports:
# put imports beginning with prefix after 3rd-party packages;
# it's a comma-separated list of prefixes
local-prefixes: opendev.org/airship/airshipctl
gocyclo:
# minimal code complexity to report, 30 by default (but we recommend 10-20)
min-complexity: 15
dupl:
# tokens count to trigger issue, 150 by default
threshold: 100
goconst:
# minimal length of string constant, 3 by default
min-len: 3
# minimal occurrences count to trigger, 3 by default
min-occurrences: 3
govet:
# report about shadowed variables
check-shadowing: true
misspell:
# Correct spellings using locale preferences for US or UK.
# Default is to use a neutral variety of English.
@ -101,47 +154,45 @@ linters-settings:
locale: US
# ignore-words:
# - someword
nakedret:
# make an issue if func has more lines of code than this setting and it has naked returns; default is 30
max-func-lines: 10
lll:
# max line length, lines longer will be reported. Default is 120.
# '\t' is counted as 1 character by default, and can be changed with the tab-width option
line-length: 120
# tab width in spaces. Default to 1.
tab-width: 1
unused:
# treat code as a program (not a library) and report unused exported identifiers; default is false.
# XXX: if you enable this setting, unused will report a lot of false-positives in text editors:
# if it's called for subdir of a project it can't find funcs usages. All text editor integrations
# with golangci-lint call it on a directory with the changed file.
check-exported: false
unparam:
# Inspect exported functions, default is false. Set to true if no external program/library imports your code.
# XXX: if you enable this setting, unparam will report a lot of false-positives in text editors:
# if it's called for subdir of a project it can't find external interfaces. All text editor integrations
# with golangci-lint call it on a directory with the changed file.
check-exported: false
nakedret:
# make an issue if func has more lines of code than this setting and it has naked returns; default is 30
max-func-lines: 10
prealloc:
# XXX: we don't recommend using this linter before doing performance profiling.
# For most programs usage of prealloc will be a premature optimization.
# Report preallocation suggestions only on simple loops that have no returns/breaks/continues/gotos in them.
# True by default.
simple: true
range-loops: true # Report preallocation suggestions on range loops, true by default
for-loops: false # Report preallocation suggestions on for loops, false by default
unused:
# treat code as a program (not a library) and report unused exported identifiers; default is false.
# XXX: if you enable this setting, unused will report a lot of false-positives in text editors:
# if it's called for subdir of a project it can't find funcs usages. All text editor integrations
# with golangci-lint call it on a directory with the changed file.
check-exported: false
linters:
disable-all: true
enable:
- dupl # Tool for code clone detection
- errcheck # Errcheck is a program for checking for unchecked errors in go programs. These unchecked errors can be critical bugs in some cases
- goconst # Finds repeated strings that could be replaced by a constant
- gocritic # The most opinionated Go source code linter
- gocyclo # Computes and checks the cyclomatic complexity of functions
- gofmt # Gofmt checks whether code was gofmt-ed. By default this tool runs with -s option to check for code simplification
- goimports # Goimports does everything that gofmt does. Additionally it checks unused imports
- gosec # Inspects source code for security problems
- govet # Vet examines Go source code and reports suspicious constructs, such as Printf calls whose arguments do not align with the format string
- ineffassign # Detects when assignments to existing variables are not used
- interfacer # Linter that suggests narrower interface types
- lll # Reports long lines
- misspell # Finds commonly misspelled English words in comments
@ -150,3 +201,6 @@ linters:
- scopelint # Scopelint checks for unpinned variables in go programs
- unconvert # Remove unnecessary type conversions
- unparam # Reports unused function parameters
- unused # Checks Go code for unused constants, variables, functions and types
- varcheck # Finds unused global variables and constants
- whitespace # Tool for detection of leading and trailing whitespace NOTE(howell): This linter does _not_ check for trailing whitespace in multiline strings

View File

@ -13,8 +13,8 @@ func NewBootstrapCommand(rootSettings *environment.AirshipCTLSettings) *cobra.Co
Short: "Bootstrap ephemeral Kubernetes cluster",
}
ISOGenCmd := NewISOGenCommand(bootstrapRootCmd, rootSettings)
bootstrapRootCmd.AddCommand(ISOGenCmd)
isoGenCmd := NewISOGenCommand(bootstrapRootCmd, rootSettings)
bootstrapRootCmd.AddCommand(isoGenCmd)
remoteDirectCmd := NewRemoteDirectCommand(rootSettings)
bootstrapRootCmd.AddCommand(remoteDirectCmd)

View File

@ -59,7 +59,6 @@ func NewRemoteDirectCommand(rootSettings *environment.AirshipCTLSettings) *cobra
Use: "remotedirect",
Short: "Bootstrap ephemeral node",
RunE: func(cmd *cobra.Command, args []string) error {
/* TODO: Get config from settings.GetCurrentContext() and remove cli arguments */
/* Trigger remotedirect based on remote type */

View File

@ -27,7 +27,6 @@ const (
)
func TestConfig(t *testing.T) {
cmdTests := []*testutil.CmdTest{
{
Name: "config-cmd-with-defaults",

View File

@ -39,7 +39,6 @@ airshipctl config get-cluster e2e --%v=ephemeral`, config.FlagClusterType)
// NewCmdConfigGetCluster returns a Command instance for 'config -Cluster' sub command
func NewCmdConfigGetCluster(rootSettings *environment.AirshipCTLSettings) *cobra.Command {
theCluster := &config.ClusterOptions{}
getclustercmd := &cobra.Command{
Use: "get-cluster NAME",

View File

@ -46,7 +46,6 @@ airshipctl config get-context e2e`,
// NewCmdConfigGetContext returns a Command instance for 'config -Context' sub command
func NewCmdConfigGetContext(rootSettings *environment.AirshipCTLSettings) *cobra.Command {
theContext := &config.ContextOptions{}
getcontextcmd := &cobra.Command{
Use: "get-context NAME",
@ -68,7 +67,6 @@ func NewCmdConfigGetContext(rootSettings *environment.AirshipCTLSettings) *cobra
func gctxInitFlags(o *config.ContextOptions, getcontextcmd *cobra.Command) {
getcontextcmd.Flags().BoolVar(&o.CurrentContext, config.FlagCurrentContext, false,
config.FlagCurrentContext+" to retrieve the current context entry in airshipctl config")
}
// runGetContext performs the execution of 'config get-Context' sub command

View File

@ -99,7 +99,6 @@ func TestNoContextsGetContextCmd(t *testing.T) {
}
func getNamedTestContext(contextName string) *config.Context {
kContext := &kubeconfig.Context{
Namespace: "dummy_namespace",
AuthInfo: "dummy_user",

View File

@ -17,12 +17,14 @@ limitations under the License.
package config
import (
"errors"
"fmt"
"github.com/spf13/cobra"
"opendev.org/airship/airshipctl/pkg/config"
"opendev.org/airship/airshipctl/pkg/environment"
conferrors "opendev.org/airship/airshipctl/pkg/errors"
"opendev.org/airship/airshipctl/pkg/log"
)
@ -87,7 +89,6 @@ func NewCmdConfigSetCluster(rootSettings *environment.AirshipCTLSettings) *cobra
}
func scInitFlags(o *config.ClusterOptions, setclustercmd *cobra.Command) {
setclustercmd.Flags().StringVar(&o.Server, config.FlagAPIServer, o.Server,
config.FlagAPIServer+" for the cluster entry in airshipctl config")
@ -106,11 +107,9 @@ func scInitFlags(o *config.ClusterOptions, setclustercmd *cobra.Command) {
setclustercmd.Flags().BoolVar(&o.EmbedCAData, config.FlagEmbedCerts, false,
config.FlagEmbedCerts+" for the cluster entry in airshipctl config")
}
func runSetCluster(o *config.ClusterOptions, rootSettings *environment.AirshipCTLSettings) (bool, error) {
clusterWasModified := false
err := o.Validate()
if err != nil {
@ -119,9 +118,14 @@ func runSetCluster(o *config.ClusterOptions, rootSettings *environment.AirshipCT
airconfig := rootSettings.Config()
cluster, err := airconfig.GetCluster(o.Name, o.ClusterType)
// Safe to ignore the error. Simple means I didnt find the cluster
if cluster == nil {
// New Cluster
if err != nil {
var cerr conferrors.ErrMissingConfig
if !errors.As(err, &cerr) {
// An error occurred, but it wasn't a "missing" config error.
return clusterWasModified, err
}
// Cluster didn't exist, create it
_, err := airconfig.AddCluster(o)
if err != nil {
return clusterWasModified, err

View File

@ -116,7 +116,6 @@ func TestSetClusterWithCAFileData(t *testing.T) {
}
func TestSetCluster(t *testing.T) {
conf := config.InitConfig(t)
tname := testCluster
@ -188,7 +187,6 @@ func TestModifyCluster(t *testing.T) {
}
func (test setClusterTest) run(t *testing.T) {
// Get the Environment
settings := &environment.AirshipCTLSettings{}
settings.SetConfig(test.config)

View File

@ -77,7 +77,6 @@ func NewCmdConfigSetContext(rootSettings *environment.AirshipCTLSettings) *cobra
}
func sctxInitFlags(o *config.ContextOptions, setcontextcmd *cobra.Command) {
setcontextcmd.Flags().BoolVar(&o.CurrentContext, config.FlagCurrentContext, false,
config.FlagCurrentContext+" for the context entry in airshipctl config")
@ -95,7 +94,6 @@ func sctxInitFlags(o *config.ContextOptions, setcontextcmd *cobra.Command) {
setcontextcmd.Flags().StringVar(&o.ClusterType, config.FlagClusterType, "",
config.FlagClusterType+" for the context entry in airshipctl config")
}
func runSetContext(o *config.ContextOptions, airconfig *config.Config) (bool, error) {

View File

@ -44,7 +44,6 @@ type setContextTest struct {
}
func TestConfigSetContext(t *testing.T) {
cmdTests := []*testutil.CmdTest{
{
Name: "config-cmd-set-context-with-help",
@ -59,7 +58,6 @@ func TestConfigSetContext(t *testing.T) {
}
func TestSetContext(t *testing.T) {
conf := config.InitConfig(t)
tname := "dummycontext"
@ -149,7 +147,6 @@ func TestModifyContext(t *testing.T) {
}
func (test setContextTest) run(t *testing.T) {
// Get the Environment
settings := &environment.AirshipCTLSettings{}
settings.SetConfig(test.config)
@ -186,5 +183,4 @@ func (test setContextTest) run(t *testing.T) {
if len(test.expected) != 0 {
assert.EqualValuesf(t, buf.String(), test.expected, "expected %v, but got %v", test.expected, buf.String())
}
}

View File

@ -1,4 +1,4 @@
Error: Cluster clusterMissing information was not found in the configuration.
Error: Missing configuration: Cluster with name 'clusterMissing' of type 'target'
Usage:
get-cluster NAME [flags]

View File

@ -1,4 +1,4 @@
Error: Missing configuration
Error: Missing configuration: Context with name 'contextMissing'
Usage:
get-context NAME [flags]

View File

@ -38,7 +38,6 @@ func NewRootCmd(out io.Writer) (*cobra.Command, *environment.AirshipCTLSettings,
SilenceUsage: true,
PersistentPreRun: func(cmd *cobra.Command, args []string) {
log.Init(settings.Debug, cmd.OutOrStderr())
},
}
rootCmd.SetOutput(out)
@ -54,7 +53,6 @@ func NewRootCmd(out io.Writer) (*cobra.Command, *environment.AirshipCTLSettings,
// AddDefaultAirshipCTLCommands is a convenience function for adding all of the
// default commands to airshipctl
func AddDefaultAirshipCTLCommands(cmd *cobra.Command, settings *environment.AirshipCTLSettings) *cobra.Command {
//cmd.AddCommand(argo.NewCommand())
cmd.AddCommand(bootstrap.NewBootstrapCommand(settings))
cmd.AddCommand(cluster.NewClusterCommand(settings))
cmd.AddCommand(completion.NewCompletionCommand())

View File

@ -11,7 +11,6 @@ import (
)
func TestGetCloudData(t *testing.T) {
fSys := testutil.SetupTestFs(t, "testdata")
bundle, err := document.NewBundle(fSys, "/", "/")
require.NoError(t, err, "Building Bundle Failed")

View File

@ -58,7 +58,6 @@ func GenerateBootstrapIso(settings *Settings, args []string) error {
}
log.Print("Checking artifacts")
return verifyArtifacts(cfg)
}
func verifyInputs(cfg *Config, args []string) error {
@ -124,6 +123,10 @@ func generateBootstrapIso(
var fls map[string][]byte
fls, err = getContainerCfg(cfg, userData, netConf)
if err != nil {
return err
}
if err = util.WriteFiles(fls, 0600); err != nil {
return err
}

View File

@ -179,5 +179,4 @@ func TestVerifyInputs(t *testing.T) {
actualErr := verifyInputs(tt.cfg, tt.args)
assert.Equal(t, tt.expectedErr, actualErr)
}
}

View File

@ -67,5 +67,4 @@ func TestValidateContext(t *testing.T) {
// Valid Data case
err := co.Validate()
assert.NoError(t, err)
}

View File

@ -240,7 +240,6 @@ func (c *Config) reconcileAuthInfos() {
if c.AuthInfos[key] == nil && authinfo != nil {
// Add the reference
c.AuthInfos[key] = NewAuthInfo()
}
}
// Checking if there is any AuthInfo reference in airship config that does not match
@ -392,14 +391,12 @@ func (c *Config) ContextNames() []string {
func (c *Config) GetCluster(cName, cType string) (*Cluster, error) {
_, exists := c.Clusters[cName]
if !exists {
return nil, errors.New("Cluster " + cName +
" information was not found in the configuration.")
return nil, conferrors.ErrMissingConfig{What: fmt.Sprintf("Cluster with name '%s' of type '%s'", cName, cType)}
}
// Alternative to this would be enhance Cluster.String() to embedd the appropriate kubeconfig cluster information
cluster, exists := c.Clusters[cName].ClusterTypes[cType]
if !exists {
return nil, errors.New("Cluster " + cName + " of type " + cType +
" information was not found in the configuration.")
return nil, conferrors.ErrMissingConfig{What: fmt.Sprintf("Cluster with name '%s' of type '%s'", cName, cType)}
}
return cluster, nil
}
@ -426,7 +423,6 @@ func (c *Config) AddCluster(theCluster *ClusterOptions) (*Cluster, error) {
// Ok , I have initialized structs for the Cluster information
// We can use Modify to populate the correct information
return c.ModifyCluster(nCluster, theCluster)
}
func (c *Config) ModifyCluster(cluster *Cluster, theCluster *ClusterOptions) (*Cluster, error) {
@ -470,8 +466,8 @@ func (c *Config) ModifyCluster(cluster *Cluster, theCluster *ClusterOptions) (*C
}
}
return cluster, nil
}
func (c *Config) GetClusters() ([]*Cluster, error) {
clusters := []*Cluster{}
for _, cName := range c.ClusterNames() {
@ -493,7 +489,7 @@ func (c *Config) GetClusters() ([]*Cluster, error) {
func (c *Config) GetContext(cName string) (*Context, error) {
context, exists := c.Contexts[cName]
if !exists {
return nil, conferrors.ErrMissingConfig{}
return nil, conferrors.ErrMissingConfig{What: fmt.Sprintf("Context with name '%s'", cName)}
}
return context, nil
}
@ -528,7 +524,6 @@ func (c *Config) AddContext(theContext *ContextOptions) *Context {
// We can use Modify to populate the correct information
c.ModifyContext(nContext, theContext)
return nContext
}
func (c *Config) ModifyContext(context *Context, theContext *ContextOptions) {
@ -596,12 +591,7 @@ func (c *Config) CurrentContextManifest() (*Manifest, error) {
// Purge removes the config file
func (c *Config) Purge() error {
//configFile := c.ConfigFile()
err := os.Remove(c.loadedConfigPath)
if err != nil {
return err
}
return nil
return os.Remove(c.loadedConfigPath)
}
func (c *Config) Equal(d *Config) bool {
@ -699,7 +689,6 @@ func (c *Context) ClusterType() string {
clusterName := NewClusterComplexName()
clusterName.FromName(c.NameInKubeconf)
return clusterName.ClusterType()
}
// AuthInfo functions

View File

@ -395,5 +395,4 @@ func TestGetContext(t *testing.T) {
// Test Wrong Cluster
_, err = conf.GetContext("unknown")
assert.Error(t, err)
}

View File

@ -93,6 +93,8 @@ func DummyManifest() *Manifest {
}
func DummyRepository() *Repository {
// TODO(howell): handle this error
//nolint: errcheck
url, _ := url.Parse("http://dummy.url.com")
return &Repository{
Url: url,
@ -121,12 +123,15 @@ func DummyClusterPurpose() *ClusterPurpose {
func InitConfig(t *testing.T) *Config {
t.Helper()
testDir, err := ioutil.TempDir("", "airship-test")
require.NoError(t, err)
configPath := filepath.Join(testDir, "config")
ioutil.WriteFile(configPath, []byte(testConfigYAML), 0666)
err = ioutil.WriteFile(configPath, []byte(testConfigYAML), 0666)
require.NoError(t, err)
kubeConfigPath := filepath.Join(testDir, "kubeconfig")
ioutil.WriteFile(kubeConfigPath, []byte(testKubeConfigYAML), 0666)
err = ioutil.WriteFile(kubeConfigPath, []byte(testKubeConfigYAML), 0666)
require.NoError(t, err)
conf := NewConfig()

View File

@ -30,8 +30,6 @@ func NewContainer(ctx *context.Context, driver string, url string) (Container, e
}
return NewDockerContainer(ctx, url, cli)
default:
return nil, ErrContainerDrvNotSupported{Driver: driver}
}
}

View File

@ -11,6 +11,7 @@ import (
"time"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"github.com/docker/docker/api/types"
"github.com/docker/docker/api/types/container"
@ -139,7 +140,6 @@ func TestGetCmd(t *testing.T) {
Cmd: []string{"testCmd"},
},
}, nil, nil
},
},
expectedResult: []string{"testCmd"},
@ -166,9 +166,7 @@ func TestGetCmd(t *testing.T) {
assert.Equal(t, tt.expectedErr, actualErr)
assert.Equal(t, tt.expectedResult, actualRes)
}
}
func TestGetImageId(t *testing.T) {
@ -242,7 +240,8 @@ func TestImagePull(t *testing.T) {
func TestGetId(t *testing.T) {
cnt := getDockerContainerMock(mockDockerClient{})
cnt.RunCommand([]string{"testCmd"}, nil, nil, []string{}, false)
err := cnt.RunCommand([]string{"testCmd"}, nil, nil, []string{}, false)
require.NoError(t, err)
actialId := cnt.GetId()
assert.Equal(t, "testID", actialId)
@ -333,7 +332,6 @@ func TestRunCommand(t *testing.T) {
debug: false,
mockDockerClient: mockDockerClient{
containerWait: func() (<-chan container.ContainerWaitOKBody, <-chan error) {
errC := make(chan error)
go func() {
errC <- containerWaitError
@ -351,7 +349,6 @@ func TestRunCommand(t *testing.T) {
debug: false,
mockDockerClient: mockDockerClient{
containerWait: func() (<-chan container.ContainerWaitOKBody, <-chan error) {
resC := make(chan container.ContainerWaitOKBody)
go func() {
resC <- container.ContainerWaitOKBody{StatusCode: 1}
@ -412,7 +409,9 @@ func TestRunCommandOutput(t *testing.T) {
var actualResBytes []byte
if actualRes != nil {
actualResBytes, _ = ioutil.ReadAll(actualRes)
var err error
actualResBytes, err = ioutil.ReadAll(actualRes)
require.NoError(t, err)
} else {
actualResBytes = []byte{}
}

View File

@ -54,8 +54,7 @@ type Bundle interface {
// NewBundle is a convenience function to create a new bundle
// Over time, it will evolve to support allowing more control
// for kustomize plugins
func NewBundle(fSys fs.FileSystem, kustomizePath string, outputPath string) (Bundle, error) {
func NewBundle(fSys fs.FileSystem, kustomizePath string, outputPath string) (bundle Bundle, err error) {
var options = KustomizeBuildOptions{
KustomizationPath: kustomizePath,
OutputPath: outputPath,
@ -64,11 +63,15 @@ func NewBundle(fSys fs.FileSystem, kustomizePath string, outputPath string) (Bun
}
// init an empty bundle factory
var bundle Bundle = &BundleFactory{}
bundle = &BundleFactory{}
// set the fs and build options we will use
bundle.SetFileSystem(fSys)
bundle.SetKustomizeBuildOptions(options)
if err = bundle.SetFileSystem(fSys); err != nil {
return nil, err
}
if err = bundle.SetKustomizeBuildOptions(options); err != nil {
return nil, err
}
// boiler plate to allow us to run Kustomize build
uf := kunstruct.NewKunstructuredFactoryImpl()
@ -84,7 +87,11 @@ func NewBundle(fSys fs.FileSystem, kustomizePath string, outputPath string) (Bun
if err != nil {
return bundle, err
}
defer ldr.Cleanup()
defer func() {
err = ldr.Cleanup()
}()
kt, err := target.NewKustTarget(ldr, rf, pf, pl)
if err != nil {
return bundle, err
@ -92,13 +99,12 @@ func NewBundle(fSys fs.FileSystem, kustomizePath string, outputPath string) (Bun
// build a resource map of kustomize rendered objects
m, err := kt.MakeCustomizedResMap()
bundle.SetKustomizeResourceMap(m)
if err != nil {
return bundle, err
}
err = bundle.SetKustomizeResourceMap(m)
return bundle, nil
return bundle, err
}
// GetKustomizeResourceMap returns a Kustomize Resource Map for this bundle
@ -175,7 +181,6 @@ func (b *BundleFactory) GetByName(name string) (Document, error) {
// Select offers a direct interface to pass a Kustomize Selector to the bundle
// returning Documents that match the criteria
func (b *BundleFactory) Select(selector types.Selector) ([]Document, error) {
// use the kustomize select method
resources, err := b.ResMap.Select(selector)
if err != nil {
@ -185,7 +190,8 @@ func (b *BundleFactory) Select(selector types.Selector) ([]Document, error) {
// Construct Bundle document for each resource returned
docSet := []Document{}
for _, res := range resources {
doc, err := NewDocument(res)
var doc Document
doc, err = NewDocument(res)
if err != nil {
return docSet, err
}
@ -196,36 +202,30 @@ func (b *BundleFactory) Select(selector types.Selector) ([]Document, error) {
// GetByAnnotation is a convenience method to get documents for a particular annotation
func (b *BundleFactory) GetByAnnotation(annotation string) ([]Document, error) {
// Construct kustomize annotation selector
selector := types.Selector{AnnotationSelector: annotation}
// pass it to the selector
return b.Select(selector)
}
// GetByLabel is a convenience method to get documents for a particular label
func (b *BundleFactory) GetByLabel(label string) ([]Document, error) {
// Construct kustomize annotation selector
selector := types.Selector{LabelSelector: label}
// pass it to the selector
return b.Select(selector)
}
// GetByGvk is a convenience method to get documents for a particular Gvk tuple
func (b *BundleFactory) GetByGvk(group, version, kind string) ([]Document, error) {
// Construct kustomize gvk object
g := gvk.Gvk{Group: group, Version: version, Kind: kind}
// pass it to the selector
selector := types.Selector{Gvk: g}
return b.Select(selector)
}
// Write will write out the entire bundle resource map

View File

@ -17,7 +17,6 @@ func TestNewBundle(t *testing.T) {
bundle := testutil.NewTestBundle(t, "testdata")
require.NotNil(bundle)
}
func TestBundleDocumentFiltering(t *testing.T) {
@ -27,43 +26,34 @@ func TestBundleDocumentFiltering(t *testing.T) {
bundle := testutil.NewTestBundle(t, "testdata")
t.Run("GetKustomizeResourceMap", func(t *testing.T) {
r := bundle.GetKustomizeResourceMap()
// ensure it is populated
assert.NotZero(r.Size())
})
t.Run("GetByGvk", func(t *testing.T) {
docs, err := bundle.GetByGvk("apps", "v1", "Deployment")
require.NoError(err)
assert.Len(docs, 3, "GetGvk returned the wrong number of resources")
})
t.Run("GetByAnnotation", func(t *testing.T) {
docs, err := bundle.GetByAnnotation("airshipit.org/clustertype=ephemeral")
require.NoError(err, "Error trying to GetByAnnotation")
assert.Len(docs, 4, "GetByAnnotation returned wrong number of resources")
})
t.Run("GetByLabel", func(t *testing.T) {
docs, err := bundle.GetByLabel("app=workflow-controller")
require.NoError(err, "Error trying to GetByLabel")
assert.Len(docs, 1, "GetByLabel returned wrong number of resources")
})
t.Run("SelectGvk", func(t *testing.T) {
// Select* tests test the Kustomize selector, which requires we build Kustomize
// selector objects which is useful for advanced searches that
// need to stack filters
@ -73,22 +63,18 @@ func TestBundleDocumentFiltering(t *testing.T) {
require.NoError(err, "Error trying to select resources")
assert.Len(docs, 3, "SelectGvk returned wrong number of resources")
})
t.Run("SelectAnnotations", func(t *testing.T) {
// Find documents with a particular annotation, namely airshipit.org/clustertype=ephemeral
selector := types.Selector{AnnotationSelector: "airshipit.org/clustertype=ephemeral"}
docs, err := bundle.Select(selector)
require.NoError(err, "Error trying to select annotated resources")
assert.Len(docs, 4, "SelectAnnotations returned wrong number of resources")
})
t.Run("SelectLabels", func(t *testing.T) {
// Find documents with a particular label, namely app=workflow-controller
// note how this will only find resources labeled at the top most level (metadata.labels)
// and not spec templates with this label (spec.template.metadata.labels)
@ -96,11 +82,9 @@ func TestBundleDocumentFiltering(t *testing.T) {
docs, err := bundle.Select(selector)
require.NoError(err, "Error trying to select labeled resources")
assert.Len(docs, 1, "SelectLabels returned wrong number of resources")
})
t.Run("Write", func(t *testing.T) {
// Ensure we can write out a bundle
//
// alanmeadows(TODO) improve validation what we write looks correct
@ -113,7 +97,5 @@ func TestBundleDocumentFiltering(t *testing.T) {
// so we for now we just search for an expected string
// obviously, this should be improved
assert.Contains(b.String(), "workflow-controller")
})
}

View File

@ -30,9 +30,8 @@ type Document interface {
// GetNamespace returns the namespace the resource thinks it's in.
func (d *DocumentFactory) GetNamespace() string {
namespace, _ := d.GetString("metadata.namespace")
// if err, namespace is empty, so no need to check.
return namespace
r := d.GetKustomizeResource()
return r.GetNamespace()
}
// GetString returns the string value at path.
@ -125,9 +124,7 @@ func (d *DocumentFactory) SetKustomizeResource(r *resource.Resource) error {
// documents - e.g. in the future all documents require an airship
// annotation X
func NewDocument(r *resource.Resource) (Document, error) {
var doc Document = &DocumentFactory{}
err := doc.SetKustomizeResource(r)
return doc, err
}

View File

@ -27,7 +27,6 @@ func TestDocument(t *testing.T) {
require.NotNil(bundle)
t.Run("GetName", func(t *testing.T) {
docs, err := bundle.GetAllDocuments()
require.NoError(err, "Unexpected error trying to GetAllDocuments")
@ -38,11 +37,9 @@ func TestDocument(t *testing.T) {
}
assert.Contains(nameList, "tiller-deploy", "Could not find expected name")
})
t.Run("AsYAML", func(t *testing.T) {
doc, err := bundle.GetByName("some-random-deployment-we-will-filter")
require.NoError(err, "Unexpected error trying to GetByName for AsYAML Test")
@ -70,11 +67,9 @@ func TestDocument(t *testing.T) {
// increase the chance of a match by removing any \n suffix on both actual
// and expected
assert.Equal(strings.TrimRight(s, "\n"), strings.TrimRight(string(fileData), "\n"))
})
t.Run("GetString", func(t *testing.T) {
doc, err := bundle.GetByName("some-random-deployment-we-will-filter")
require.NoError(err, "Unexpected error trying to GetByName")
@ -82,20 +77,16 @@ func TestDocument(t *testing.T) {
require.NoError(err, "Unexpected error trying to GetString from document")
assert.Equal(appLabelMatch, "some-random-deployment-we-will-filter")
})
t.Run("GetNamespace", func(t *testing.T) {
doc, err := bundle.GetByName("some-random-deployment-we-will-filter")
require.NoError(err, "Unexpected error trying to GetByName")
assert.Equal("foobar", doc.GetNamespace())
})
t.Run("GetStringSlice", func(t *testing.T) {
doc, err := bundle.GetByName("some-random-deployment-we-will-filter")
require.NoError(err, "Unexpected error trying to GetByName")
s := []string{"foobar"}
@ -104,7 +95,5 @@ func TestDocument(t *testing.T) {
require.NoError(err, "Unexpected error trying to GetStringSlice")
assert.Equal(s, gotSlice)
})
}

View File

@ -50,7 +50,6 @@ type Repository struct {
// NewRepository create repository object, with real filesystem on disk
// basePath is used to calculate final path where to clone/open the repository
func NewRepository(basePath string, builder OptionsBuilder) (*Repository, error) {
dirName := nameFromURL(builder.URL())
if dirName == "" {
return nil, fmt.Errorf("URL: %s, original error: %w", builder.URL(), ErrCantParseUrl)

View File

@ -46,8 +46,8 @@ func TestNewRepositoryNegative(t *testing.T) {
assert.Error(t, err)
assert.Nil(t, repo)
}
func TestDownload(t *testing.T) {
func TestDownload(t *testing.T) {
err := fixtures.Init()
require.NoError(t, err)
fx := fixtures.Basic().One()
@ -134,7 +134,7 @@ func TestUpdate(t *testing.T) {
err = repo.Checkout(true)
assert.NoError(t, err)
// update repository
err = repo.Update(true)
require.NoError(t, repo.Update(true))
currentHash, err := repo.Driver.Head()
assert.NoError(t, err)
@ -144,7 +144,6 @@ func TestUpdate(t *testing.T) {
repo.Driver.Close()
updateError := repo.Update(true)
assert.Error(t, updateError)
}
func TestOpen(t *testing.T) {
@ -161,11 +160,10 @@ func TestOpen(t *testing.T) {
repo, err := NewRepository(".", builder)
require.NoError(t, err)
driver := &GitDriver{
repo.Driver = &GitDriver{
Filesystem: memfs.New(),
Storer: memory.NewStorage(),
}
repo.Driver = driver
err = repo.Clone()
assert.NotNil(t, repo.Driver)
@ -173,7 +171,17 @@ func TestOpen(t *testing.T) {
// This should open the repo
repoOpen, err := NewRepository(".", builder)
err = repoOpen.Open()
require.NoError(t, err)
storer := memory.NewStorage()
err = storer.SetReference(plumbing.NewReferenceFromStrings("HEAD", ""))
require.NoError(t, err)
repoOpen.Driver = &GitDriver{
Filesystem: memfs.New(),
Storer: storer,
}
require.NoError(t, repoOpen.Open())
ref, err := repo.Driver.Head()
assert.NoError(t, err)
assert.NotNil(t, ref.String())

View File

@ -31,10 +31,11 @@ func (e ErrWrongConfig) Error() string {
// ErrMissingConfig returned in case of missing configuration
type ErrMissingConfig struct {
What string
}
func (e ErrMissingConfig) Error() string {
return "Missing configuration"
return "Missing configuration: " + e.What
}
// ErrConfigFailed returned in case of failure during configuration

View File

@ -30,7 +30,6 @@ type RedfishRemoteDirect struct {
// Top level function to handle Redfish remote direct
func (cfg RedfishRemoteDirect) DoRemoteDirect() error {
alog.Debugf("Using Redfish Endpoint: '%s'", cfg.RemoteURL.String())
/* TODO: Add Authentication when redfish library supports it. */
@ -44,10 +43,7 @@ func (cfg RedfishRemoteDirect) DoRemoteDirect() error {
alog.Debugf("Ephemeral Node System ID: '%s'", systemId)
/* get manager for system */
managerId, err := GetResourceIDFromURL(system.Links.ManagedBy[0].OdataId)
if err != nil {
return err
}
managerId := GetResourceIDFromURL(system.Links.ManagedBy[0].OdataId)
alog.Debugf("Ephemeral node managerId: '%s'", managerId)
/* Get manager's Cd or DVD virtual media ID */
@ -86,7 +82,6 @@ func NewRedfishRemoteDirectClient(ctx context.Context,
ephNodeID string,
isoPath string,
) (RedfishRemoteDirect, error) {
if remoteURL == "" {
return RedfishRemoteDirect{},
NewRedfishConfigErrorf("redfish remote url empty")

View File

@ -22,7 +22,6 @@ const (
)
func TestRedfishRemoteDirectNormal(t *testing.T) {
m := &redfishMocks.RedfishAPI{}
defer m.AssertExpectations(t)
@ -60,7 +59,6 @@ func TestRedfishRemoteDirectNormal(t *testing.T) {
}
func TestRedfishRemoteDirectInvalidSystemId(t *testing.T) {
m := &redfishMocks.RedfishAPI{}
defer m.AssertExpectations(t)
@ -81,7 +79,6 @@ func TestRedfishRemoteDirectInvalidSystemId(t *testing.T) {
}
func TestRedfishRemoteDirectGetSystemNetworkError(t *testing.T) {
m := &redfishMocks.RedfishAPI{}
defer m.AssertExpectations(t)
@ -103,7 +100,6 @@ func TestRedfishRemoteDirectGetSystemNetworkError(t *testing.T) {
}
func TestRedfishRemoteDirectInvalidIsoPath(t *testing.T) {
m := &redfishMocks.RedfishAPI{}
defer m.AssertExpectations(t)
@ -131,7 +127,6 @@ func TestRedfishRemoteDirectInvalidIsoPath(t *testing.T) {
}
func TestRedfishRemoteDirectCdDvdNotAvailableInBootSources(t *testing.T) {
m := &redfishMocks.RedfishAPI{}
defer m.AssertExpectations(t)
@ -157,7 +152,6 @@ func TestRedfishRemoteDirectCdDvdNotAvailableInBootSources(t *testing.T) {
}
func TestRedfishRemoteDirectSetSystemBootSourceFailed(t *testing.T) {
m := &redfishMocks.RedfishAPI{}
defer m.AssertExpectations(t)
@ -185,7 +179,6 @@ func TestRedfishRemoteDirectSetSystemBootSourceFailed(t *testing.T) {
}
func TestRedfishRemoteDirectSystemRebootFailed(t *testing.T) {
m := &redfishMocks.RedfishAPI{}
defer m.AssertExpectations(t)

View File

@ -8,6 +8,8 @@ import (
redfishApi "github.com/Nordix/go-redfish/api"
redfishClient "github.com/Nordix/go-redfish/client"
"opendev.org/airship/airshipctl/pkg/log"
)
const (
@ -17,24 +19,23 @@ const (
// Redfish Id ref is a URI which contains resource Id
// as the last part. This function extracts resource
// ID from ID ref
func GetResourceIDFromURL(refURL string) (string, error) {
func GetResourceIDFromURL(refURL string) string {
u, err := url.Parse(refURL)
if err != nil {
return "", err
log.Fatal(err)
}
url := strings.TrimSuffix(u.Path, "/")
elems := strings.Split(url, "/")
id := elems[len(elems)-1]
return id, nil
return id
}
// Checks whether an ID exists in Redfish IDref collection
func IsIDInList(idRefList []redfishClient.IdRef, id string) bool {
for _, r := range idRefList {
rId, _ := GetResourceIDFromURL(r.OdataId)
rId := GetResourceIDFromURL(r.OdataId)
if rId == id {
return true
}
@ -48,7 +49,6 @@ func GetVirtualMediaId(ctx context.Context,
api redfishApi.RedfishAPI,
managerId string,
) (string, string, error) {
// TODO: Sushy emulator has a bug which sends 'virtualMedia.inserted' field as
// string instead of a boolean which causes the redfish client to fail
// while unmarshalling this field.
@ -62,7 +62,6 @@ func SetSystemBootSourceForMediaType(ctx context.Context,
api redfishApi.RedfishAPI,
systemId string,
mediaType string) error {
/* Check available boot sources for system */
system, _, err := api.GetSystem(ctx, systemId)
if err != nil {
@ -85,7 +84,6 @@ func SetSystemBootSourceForMediaType(ctx context.Context,
// Reboots a system by force shutoff and turning on.
func RebootSystem(ctx context.Context, api redfishApi.RedfishAPI, systemId string) error {
resetReq := redfishClient.ResetRequestBody{}
resetReq.ResetType = redfishClient.RESETTYPE_FORCE_OFF
_, httpResp, err := api.ResetSystem(ctx, systemId, resetReq)
@ -109,7 +107,6 @@ func SetVirtualMedia(ctx context.Context,
managerId string,
vMediaId string,
isoPath string) error {
vMediaReq := redfishClient.InsertMediaRequestBody{}
vMediaReq.Image = isoPath
vMediaReq.Inserted = true

View File

@ -16,13 +16,11 @@ const (
)
func TestRedfishErrorNoError(t *testing.T) {
err := ScreenRedfishError(nil, nil)
assert.NoError(t, err)
}
func TestRedfishErrorNonNilErrorWithoutHttpResp(t *testing.T) {
realErr := fmt.Errorf("Sample error")
err := ScreenRedfishError(nil, realErr)
assert.Error(t, err)
@ -31,7 +29,6 @@ func TestRedfishErrorNonNilErrorWithoutHttpResp(t *testing.T) {
}
func TestRedfishErrorNonNilErrorWithHttpRespError(t *testing.T) {
realErr := fmt.Errorf("Sample error")
httpResp := &http.Response{StatusCode: 408}
@ -48,7 +45,6 @@ func TestRedfishErrorNonNilErrorWithHttpRespError(t *testing.T) {
}
func TestRedfishErrorNonNilErrorWithHttpRespOK(t *testing.T) {
realErr := fmt.Errorf("Sample error")
httpResp := &http.Response{StatusCode: 204}
@ -65,28 +61,23 @@ func TestRedfishErrorNonNilErrorWithHttpRespOK(t *testing.T) {
}
func TestRedfishUtilGetResIDFromURL(t *testing.T) {
// simple case
url := "api/user/123"
id, err := GetResourceIDFromURL(url)
assert.NoError(t, err)
id := GetResourceIDFromURL(url)
assert.Equal(t, id, "123")
// FQDN
url = "http://www.abc.com/api/user/123"
id, err = GetResourceIDFromURL(url)
assert.NoError(t, err)
id = GetResourceIDFromURL(url)
assert.Equal(t, id, "123")
//Trailing slash
url = "api/user/123/"
id, err = GetResourceIDFromURL(url)
assert.NoError(t, err)
id = GetResourceIDFromURL(url)
assert.Equal(t, id, "123")
}
func TestRedfishUtilIsIdInList(t *testing.T) {
idList := []redfishClient.IdRef{
{OdataId: "/path/to/id/1"},
{OdataId: "/path/to/id/2"},
@ -106,7 +97,6 @@ func TestRedfishUtilIsIdInList(t *testing.T) {
}
func TestRedfishUtilRebootSystemOK(t *testing.T) {
m := &redfishMocks.RedfishAPI{}
defer m.AssertExpectations(t)
@ -127,7 +117,6 @@ func TestRedfishUtilRebootSystemOK(t *testing.T) {
}
func TestRedfishUtilRebootSystemForceOffError2(t *testing.T) {
m := &redfishMocks.RedfishAPI{}
defer m.AssertExpectations(t)
@ -148,7 +137,6 @@ func TestRedfishUtilRebootSystemForceOffError2(t *testing.T) {
}
func TestRedfishUtilRebootSystemForceOffError(t *testing.T) {
m := &redfishMocks.RedfishAPI{}
defer m.AssertExpectations(t)
@ -168,7 +156,6 @@ func TestRedfishUtilRebootSystemForceOffError(t *testing.T) {
}
func TestRedfishUtilRebootSystemTurningOnError(t *testing.T) {
m := &redfishMocks.RedfishAPI{}
defer m.AssertExpectations(t)

View File

@ -41,7 +41,6 @@ type RemoteDirectClient interface {
// Get remotedirect client based on config
func getRemoteDirectClient(remoteConfig RemoteDirectConfig) (RemoteDirectClient, error) {
var client RemoteDirectClient
var err error
@ -68,7 +67,6 @@ func getRemoteDirectClient(remoteConfig RemoteDirectConfig) (RemoteDirectClient,
// Top level function to execute remote direct based on remote type
func DoRemoteDirect(remoteConfig RemoteDirectConfig) error {
client, err := getRemoteDirectClient(remoteConfig)
if err != nil {
return err

View File

@ -9,7 +9,6 @@ import (
)
func TestUnknownRemoteType(t *testing.T) {
rdCfg := RemoteDirectConfig{
RemoteType: "new-remote",
RemoteURL: "http://localhost:8000",
@ -24,7 +23,6 @@ func TestUnknownRemoteType(t *testing.T) {
}
func TestRedfishRemoteDirectWithBogusConfig(t *testing.T) {
rdCfg := RemoteDirectConfig{
RemoteType: "redfish",
RemoteURL: "http://nolocalhost:8888",
@ -39,7 +37,6 @@ func TestRedfishRemoteDirectWithBogusConfig(t *testing.T) {
}
func TestRedfishRemoteDirectWithEmptyURL(t *testing.T) {
rdCfg := RemoteDirectConfig{
RemoteType: "redfish",
RemoteURL: "",
@ -54,7 +51,6 @@ func TestRedfishRemoteDirectWithEmptyURL(t *testing.T) {
}
func TestRedfishRemoteDirectWithEmptyNodeID(t *testing.T) {
rdCfg := RemoteDirectConfig{
RemoteType: "redfish",
RemoteURL: "http://nolocalhost:8888",
@ -69,7 +65,6 @@ func TestRedfishRemoteDirectWithEmptyNodeID(t *testing.T) {
}
func TestRedfishRemoteDirectWithEmptyIsoPath(t *testing.T) {
rdCfg := RemoteDirectConfig{
RemoteType: "redfish",
RemoteURL: "http://nolocalhost:8888",

View File

@ -13,5 +13,4 @@ func WriteFiles(fls map[string][]byte, mode os.FileMode) error {
}
}
return nil
}

View File

@ -16,7 +16,6 @@ const (
// WriteOut dumps any yaml competible document to writer, adding yaml separator `---`
// at the beginning of the document, and `...` at the end
func WriteOut(dst io.Writer, src interface{}) error {
yamlOut, err := yaml.Marshal(src)
if err != nil {
return err

View File

@ -34,7 +34,6 @@ func (f *FakeErrorDashWriter) Write(b []byte) (int, error) {
}
func TestWriteOut(t *testing.T) {
// Create some object, that can be marshaled into yaml
ob := &metav1.ObjectMeta{
Name: "RandomName",
@ -93,6 +92,5 @@ func TestWriteOutErrorsErrorWriter(t *testing.T) {
for _, str := range strings {
fakeWriter.Match = str
assert.Error(t, utilyaml.WriteOut(fakeWriter, fakeYaml))
}
}