Skip to content

Commit 5bcd109

Browse files
committed
Incorporated @ncdc feedback.
1 parent 48cfe2d commit 5bcd109

File tree

2 files changed

+103
-124
lines changed

2 files changed

+103
-124
lines changed

cmd/clusterctl/pkg/client/repository/repository_local_unix.go

+42-56
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ import (
3131
// localRepository provides support for providers located on the local filesystem.
3232
// As part of the provider object, the URL is expected to contain the absolute
3333
// path to the components yaml on the local filesystem.
34-
// In order to support different versions, the directories containing provider
34+
// To support different versions, the directories containing provider
3535
// specific data must adhere to the following layout:
3636
// {basepath}/{provider-name}/{version}/{components.yaml}
3737
//
@@ -44,8 +44,7 @@ import (
4444
// basepath: /home/user/go/src/sigs.k8s.io
4545
// provider-name: aws
4646
// version: v0.4.7
47-
// component.yaml: infrastructure-components.yaml
48-
47+
// components.yaml: infrastructure-components.yaml
4948
type localRepository struct {
5049
providerConfig config.Provider
5150
configVariablesClient config.VariablesClient
@@ -57,22 +56,22 @@ type localRepository struct {
5756

5857
var _ Repository = &localRepository{}
5958

60-
// DefaultVersion returns defaultVersion field of localRepository struct
59+
// DefaultVersion returns the default version for the local repository.
6160
func (r *localRepository) DefaultVersion() string {
6261
return r.defaultVersion
6362
}
6463

65-
// RootPath returns rootPath field of localRepository struct
64+
// RootPath returns the empty string as it is not applicable to local repositories.
6665
func (r *localRepository) RootPath() string {
67-
return "" // Not applicable with localRepository
66+
return ""
6867
}
6968

70-
// ComponentsPath returns componentsPath field of localRepository struct
69+
// ComponentsPath returns the path to the components file for the local repository.
7170
func (r *localRepository) ComponentsPath() string {
7271
return r.componentsPath
7372
}
7473

75-
// GetFile returns a file for a given provider version
74+
// GetFile returns a file for a given provider version.
7675
func (r *localRepository) GetFile(version, fileName string) ([]byte, error) {
7776
var err error
7877

@@ -87,34 +86,51 @@ func (r *localRepository) GetFile(version, fileName string) ([]byte, error) {
8786

8887
absolutePath := filepath.Join(r.basepath, r.providerName, version, r.RootPath(), fileName)
8988

90-
if f, err := os.Stat(absolutePath); err == nil {
91-
if f.IsDir() {
92-
return nil, errors.Errorf("invalid path: file %q is actually a directory %q", fileName, absolutePath)
93-
}
94-
content, err := ioutil.ReadFile(absolutePath)
95-
if err != nil {
96-
return nil, errors.Wrapf(err, "failed to read files from local release %s", version)
97-
}
98-
return content, nil
89+
f, err := os.Stat(absolutePath)
90+
if err != nil {
91+
return nil, errors.Errorf("failed to read file %q from local release %s", absolutePath, version)
92+
}
93+
if f.IsDir() {
94+
return nil, errors.Errorf("invalid path: file %q is actually a directory %q", fileName, absolutePath)
95+
}
96+
content, err := ioutil.ReadFile(absolutePath)
97+
if err != nil {
98+
return nil, errors.Wrapf(err, "failed to read file %q from local release %s", absolutePath, version)
9999
}
100-
return nil, errors.Errorf("failed to read files from local release %s, path %s", version, absolutePath)
100+
return content, nil
101101

102102
}
103103

104-
// GetVersion returns the list of versions that are available for a local repository
104+
// GetVersions returns the list of versions that are available for a local repository.
105105
func (r *localRepository) GetVersions() ([]string, error) {
106-
versions, err := r.getVersions()
106+
// get all the sub-directories under {basepath}/{provider-name}/
107+
releasesPath := filepath.Join(r.basepath, r.providerName)
108+
files, err := ioutil.ReadDir(releasesPath)
107109
if err != nil {
108-
return nil, errors.Wrapf(err, "failed to get local repository versions")
110+
return nil, errors.Wrap(err, "failed to list release directories")
111+
}
112+
versions := []string{}
113+
for _, f := range files {
114+
if !f.IsDir() {
115+
continue
116+
}
117+
r := f.Name()
118+
sv, err := version.ParseSemantic(r)
119+
if err != nil {
120+
// discard releases with tags that are not a valid semantic versions (the user can point explicitly to such releases)
121+
continue
122+
}
123+
if sv.PreRelease() != "" || sv.BuildMetadata() != "" {
124+
// discard pre-releases or build releases (the user can point explicitly to such releases)
125+
continue
126+
}
127+
versions = append(versions, r)
109128
}
110129
return versions, nil
111130
}
112131

113-
// newLocalRepository returns a localRepository implementation
132+
// newLocalRepository returns a new localRepository.
114133
func newLocalRepository(providerConfig config.Provider, configVariablesClient config.VariablesClient) (*localRepository, error) {
115-
116-
var err error
117-
118134
url, err := url.Parse(providerConfig.URL())
119135
if err != nil {
120136
return nil, errors.Wrap(err, "invalid url")
@@ -167,14 +183,12 @@ func newLocalRepository(providerConfig config.Provider, configVariablesClient co
167183
return nil, errors.Wrap(err, "failed to get latest version")
168184
}
169185
}
170-
171186
return repo, nil
172187
}
173188

174189
// getLatestRelease returns the latest release for the local repository.
175190
func (r *localRepository) getLatestRelease() (string, error) {
176-
177-
versions, err := r.getVersions()
191+
versions, err := r.GetVersions()
178192
if err != nil {
179193
return "", errors.Wrapf(err, "failed to get local repository versions")
180194
}
@@ -195,31 +209,3 @@ func (r *localRepository) getLatestRelease() (string, error) {
195209
}
196210
return latestTag, nil
197211
}
198-
199-
// getVersions returns all the release versions for a local repository
200-
func (r *localRepository) getVersions() ([]string, error) {
201-
// get all the sub-directories under {basepath}/{provider-name}/
202-
releasesPath := filepath.Join(r.basepath, r.providerName)
203-
files, err := ioutil.ReadDir(releasesPath)
204-
if err != nil {
205-
return nil, errors.Wrap(err, "failed to list release directories")
206-
}
207-
versions := []string{}
208-
for _, f := range files {
209-
if !f.IsDir() {
210-
continue
211-
}
212-
r := f.Name()
213-
sv, err := version.ParseSemantic(r)
214-
if err != nil {
215-
// discard releases with tags that are not a valid semantic versions (the user can point explicitly to such releases)
216-
continue
217-
}
218-
if sv.PreRelease() != "" || sv.BuildMetadata() != "" {
219-
// discard pre-releases or build releases (the user can point explicitly to such releases)
220-
continue
221-
}
222-
versions = append(versions, r)
223-
}
224-
return versions, nil
225-
}

0 commit comments

Comments
 (0)