more work on PR reviews

pkg/acls/s3/storage.go

@@ -29,14 +29,16 @@ import (
 	"k8s.io/kops/util/pkg/vfs"
 )
 
-// s3PublicAclStrategy is the AclStrategy for objects written to the assets FileRepository
+// s3PublicAclStrategy is the AclStrategy for objects that are written with public read only ACL.
+// This strategy is used by custom file assets.
 type s3PublicAclStrategy struct {
 }
 
 var _ acls.ACLStrategy = &s3PublicAclStrategy{}
 
 // GetACL creates a s3PublicAclStrategy object for writing public files with assets FileRepository.
-// This func does not allow for read only in the state store.
+// This strategy checks if the files are inside the state store, and if the files are located inside
+// the state store, this returns nil and logs a message (level 8) that it will not run.
 func (s *s3PublicAclStrategy) GetACL(p vfs.Path, cluster *kops.Cluster) (vfs.ACL, error) {
 	if cluster.Spec.Assets == nil || cluster.Spec.Assets.FileRepository == nil {
 		return nil, nil
@@ -54,8 +56,9 @@ func (s *s3PublicAclStrategy) GetACL(p vfs.Path, cluster *kops.Cluster) (vfs.ACL
 		return "", fmt.Errorf("unable to parse: %q", fileRepository)
 	}
 
-	// I am not a huge fan of this, but it does work
-	// we are checking that the file is in s3.amazonaws.com meaning that it is in s3
+	// We are checking that the file is in s3.amazonaws.com meaning that it is in s3
+	// This will miss edge cases where the region url is used, but maintaining that code is
+	// not something I am willing to do.
 	if u.Host != "s3.amazonaws.com" {
 		return nil, nil
 	}

pkg/assets/builder.go

@@ -161,7 +161,7 @@ func (a *AssetBuilder) RemapFileAndSHA(fileURL *url.URL) (*url.URL, *hashing.Has
 
 		normalizedFileURL, err := a.normalizeURL(fileURL)
 		if err != nil {
-			return nil, nil, fmt.Errorf("unable to parse file url %q: %v", normalizedFileURL, err)
+			return nil, nil, err
 		}
 
 		fileAsset.FileURL = normalizedFileURL
@@ -199,7 +199,7 @@ func (a *AssetBuilder) RemapFileAndSHAValue(fileURL *url.URL, shaValue string) (
 
 		normalizedFile, err := a.normalizeURL(fileURL)
 		if err != nil {
-			return nil, fmt.Errorf("unable to normalize file url %q: %v", fileURL.String(), err)
+			return nil, err
 		}
 
 		fileAsset.FileURL = normalizedFile
@@ -268,7 +268,7 @@ func (a *AssetBuilder) normalizeURL(file *url.URL) (*url.URL, error) {
 
 	fileRepo, err := url.Parse(f)
 	if err != nil {
-		return nil, fmt.Errorf("unable to parse file repository %q: %v", values.StringValue(a.AssetsLocation.FileRepository), err)
+		return nil, fmt.Errorf("unable to parse file repository URL %q: %v", values.StringValue(a.AssetsLocation.FileRepository), err)
 	}
 
 	fileRepo.Path = path.Join(fileRepo.Path, file.Path)