Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Automated cherry pick of #6420: Refactor names of URLs in assets to clarify their purpose #6980

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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