Skip to content

Commit

Permalink
Refactor names of URLs in assets to clarify their purpose
Browse files Browse the repository at this point in the history
  • Loading branch information
justinsb committed Apr 23, 2019
1 parent 36b12a3 commit 6656d94
Show file tree
Hide file tree
Showing 3 changed files with 37 additions and 40 deletions.
55 changes: 26 additions & 29 deletions pkg/assets/builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -64,10 +64,10 @@ type ContainerAsset struct {

// FileAsset models a file's location.
type FileAsset struct {
// FileURL is the URL of a file that is accessed by a Kubernetes cluster.
FileURL *url.URL
// CanonicalFileURL is the source URL of a file. This is used to copy a file to a FileRepository.
CanonicalFileURL *url.URL
// DownloadURL is the URL from which the cluster should download the asset.
DownloadURL *url.URL
// CanonicalURL is the canonical location of the asset, for example as distributed by the kops project
CanonicalURL *url.URL
// SHAValue is the SHA hash of the FileAsset.
SHAValue string
}
Expand Down Expand Up @@ -208,18 +208,18 @@ func (a *AssetBuilder) RemapFileAndSHA(fileURL *url.URL) (*url.URL, *hashing.Has
}

fileAsset := &FileAsset{
FileURL: fileURL,
DownloadURL: fileURL,
}

if a.AssetsLocation != nil && a.AssetsLocation.FileRepository != nil {
fileAsset.CanonicalFileURL = fileURL
fileAsset.CanonicalURL = fileURL

normalizedFileURL, err := a.normalizeURL(fileURL)
normalizedFileURL, err := a.remapURL(fileURL)
if err != nil {
return nil, nil, err
}

fileAsset.FileURL = normalizedFileURL
fileAsset.DownloadURL = normalizedFileURL

glog.V(4).Infof("adding remapped file: %+v", fileAsset)
}
Expand All @@ -233,7 +233,7 @@ func (a *AssetBuilder) RemapFileAndSHA(fileURL *url.URL) (*url.URL, *hashing.Has
a.FileAssets = append(a.FileAssets, fileAsset)
glog.V(8).Infof("adding file: %+v", fileAsset)

return fileAsset.FileURL, h, nil
return fileAsset.DownloadURL, h, nil
}

// TODO - remove this method as CNI does now have a SHA file
Expand All @@ -245,25 +245,25 @@ func (a *AssetBuilder) RemapFileAndSHAValue(fileURL *url.URL, shaValue string) (
}

fileAsset := &FileAsset{
FileURL: fileURL,
SHAValue: shaValue,
DownloadURL: fileURL,
SHAValue: shaValue,
}

if a.AssetsLocation != nil && a.AssetsLocation.FileRepository != nil {
fileAsset.CanonicalFileURL = fileURL
fileAsset.CanonicalURL = fileURL

normalizedFile, err := a.normalizeURL(fileURL)
normalizedFile, err := a.remapURL(fileURL)
if err != nil {
return nil, err
}

fileAsset.FileURL = normalizedFile
glog.V(4).Infof("adding remapped file: %q", fileAsset.FileURL.String())
fileAsset.DownloadURL = normalizedFile
glog.V(4).Infof("adding remapped file: %q", fileAsset.DownloadURL.String())
}

a.FileAssets = append(a.FileAssets, fileAsset)

return fileAsset.FileURL, nil
return fileAsset.DownloadURL, nil
}

// FindHash returns the hash value of a FileAsset.
Expand All @@ -281,9 +281,9 @@ func (a *AssetBuilder) findHash(file *FileAsset) (*hashing.Hash, error) {
// TLDR; we use the file.CanonicalFileURL during assets phase, and use file.FileUrl the
// rest of the time. If not we get a chicken and the egg problem where we are reading the sha file
// before it exists.
u := file.FileURL
if a.Phase == "assets" && file.CanonicalFileURL != nil {
u = file.CanonicalFileURL
u := file.DownloadURL
if a.Phase == "assets" && file.CanonicalURL != nil {
u = file.CanonicalURL
}

if u == nil {
Expand Down Expand Up @@ -311,24 +311,21 @@ func (a *AssetBuilder) findHash(file *FileAsset) (*hashing.Hash, error) {
return nil, fmt.Errorf("cannot determine hash for %q (have you specified a valid file location?)", u)
}

func (a *AssetBuilder) normalizeURL(file *url.URL) (*url.URL, error) {

if a.AssetsLocation == nil || a.AssetsLocation.FileRepository == nil {
return nil, fmt.Errorf("assetLocation and fileRepository cannot be nil to normalize an file asset URL")
func (a *AssetBuilder) remapURL(canonicalURL *url.URL) (*url.URL, error) {
f := ""
if a.AssetsLocation != nil {
f = values.StringValue(a.AssetsLocation.FileRepository)
}

f := values.StringValue(a.AssetsLocation.FileRepository)

if f == "" {
return nil, fmt.Errorf("assetsLocation fileRepository cannot be an empty string")
return nil, fmt.Errorf("assetsLocation.fileRepository must be set to remap asset %v", canonicalURL)
}

fileRepo, err := url.Parse(f)
if err != nil {
return nil, fmt.Errorf("unable to parse file repository URL %q: %v", values.StringValue(a.AssetsLocation.FileRepository), err)
return nil, fmt.Errorf("unable to parse assetsLocation.fileRepository %q: %v", f, err)
}

fileRepo.Path = path.Join(fileRepo.Path, file.Path)
fileRepo.Path = path.Join(fileRepo.Path, canonicalURL.Path)

return fileRepo, nil
}
14 changes: 7 additions & 7 deletions upup/pkg/fi/cloudup/loader.go
Original file line number Diff line number Diff line change
Expand Up @@ -227,23 +227,23 @@ func (l *Loader) addAssetCopyTasks(assets []*assets.ContainerAsset, lifecycle *f
func (l *Loader) addAssetFileCopyTasks(assets []*assets.FileAsset, lifecycle *fi.Lifecycle) error {
for _, asset := range assets {

if asset.FileURL == nil {
if asset.DownloadURL == nil {
return fmt.Errorf("asset file url cannot be nil")
}

// test if the asset needs to be copied
if asset.CanonicalFileURL != nil && asset.FileURL.String() != asset.CanonicalFileURL.String() {
glog.V(10).Infof("processing asset: %q, %q", asset.FileURL.String(), asset.CanonicalFileURL.String())
if asset.CanonicalURL != nil && asset.DownloadURL.String() != asset.CanonicalURL.String() {
glog.V(10).Infof("processing asset: %q, %q", asset.DownloadURL.String(), asset.CanonicalURL.String())
context := &fi.ModelBuilderContext{
Tasks: l.tasks,
}

glog.V(10).Infof("adding task: %q", asset.FileURL.String())
glog.V(10).Infof("adding task: %q", asset.DownloadURL.String())

copyFileTask := &assettasks.CopyFile{
Name: fi.String(asset.CanonicalFileURL.String()),
TargetFile: fi.String(asset.FileURL.String()),
SourceFile: fi.String(asset.CanonicalFileURL.String()),
Name: fi.String(asset.CanonicalURL.String()),
TargetFile: fi.String(asset.DownloadURL.String()),
SourceFile: fi.String(asset.CanonicalURL.String()),
SHA: fi.String(asset.SHAValue),
Lifecycle: lifecycle,
}
Expand Down
8 changes: 4 additions & 4 deletions upup/pkg/fi/dryrun_target.go
Original file line number Diff line number Diff line change
Expand Up @@ -261,10 +261,10 @@ func (t *DryRunTarget) PrintReport(taskMap map[string]Task, out io.Writer) error
if len(t.assetBuilder.FileAssets) != 0 {
glog.V(4).Infof("FileAssets:")
for _, a := range t.assetBuilder.FileAssets {
if a.FileURL != nil && a.CanonicalFileURL != nil {
glog.V(4).Infof(" %s %s", a.FileURL.String(), a.CanonicalFileURL.String())
} else if a.FileURL != nil {
glog.V(4).Infof(" %s", a.FileURL.String())
if a.DownloadURL != nil && a.CanonicalURL != nil {
glog.V(4).Infof(" %s %s", a.DownloadURL.String(), a.CanonicalURL.String())
} else if a.DownloadURL != nil {
glog.V(4).Infof(" %s", a.DownloadURL.String())
}
}
}
Expand Down

0 comments on commit 6656d94

Please sign in to comment.