Merge pull request #18661 from hakman/hakman-aws-s3-nodeup-download

aws: download nodeup from private S3 buckets with curl

Author: kubernetes-prow[bot]

docs/operations/asset-repository.md

@@ -41,9 +41,11 @@ spec:
     fileRepository: https://example.com/files
 ```
 
-The repository must allow nodes to perform unauthenticated reads. The repository can
-be public or it can allow read access through network connectivity, such as access
-through a particular AWS Endpoint.
+For an `http://` or `https://` repository, nodes must be able to read without credentials.
+The repository can be public or allow access through network connectivity, such as a
+particular cloud endpoint.
+
+{{ kops_feature_table(kops_added_default='1.37') }}
 
 On GCE, the repository can also be a `gs://` URL. Nodes then read it with the credentials of
 their service account, which allows the bucket to be private. The service accounts of the
@@ -55,6 +57,20 @@ spec:
     fileRepository: gs://example-bucket/files
 ```
 
+{{ kops_feature_table(kops_added_default='1.37') }}
+
+On AWS, the repository can also be an `s3://` URL. Nodes then read it with the credentials of
+their instance profile, which allows the bucket to be private. The instance profiles of the
+instance groups have to be granted `s3:GetObject` on that bucket. This is supported in the
+commercial AWS and AWS GovCloud partitions. Downloading nodeup itself from an `s3://`
+`KOPS_BASE_URL` additionally requires curl 8.0 or newer on the node image.
+
+```yaml
+spec:
+  assets:
+    fileRepository: s3://example-bucket/files
+```
+
 ## Copying assets into repositories
 
 {{ kops_feature_table(kops_added_default='1.22') }}
@@ -65,7 +81,7 @@ When running `kops get assets --copy`, kOps copies assets into their respective
 they do not already exist there.
 
 For file assets, kOps only supports copying to a repository that is either an S3 or GCS bucket.
-An S3 bucket must be configured using the [regional naming conventions of S3](https://docs.aws.amazon.com/general/latest/gr/rande.html#s3_region).
+An S3 bucket must be configured with a prefix of `s3://` or using the [regional naming conventions of S3](https://docs.aws.amazon.com/general/latest/gr/rande.html#s3_region).
 A GCS bucket must be configured with a prefix of `https://storage.googleapis.com/` or `gs://`.
 
 ## Listing assets

docs/releases/1.37-NOTES.md

@@ -32,6 +32,8 @@ As part of this removal, the `protokube` component, whose only remaining respons
 
 * On AWS, instance roles that require no permissions, such as the bastion role and the default worker node role, no longer have an inline IAM policy. `kops update cluster` deletes the previously created inline policy from such roles. With the Terraform target, the corresponding `aws_iam_role_policy` resources are removed from the configuration and destroyed on the next apply.
 
+* Private cluster asset repositories now support AWS `s3://` URLs in addition to existing GCE `gs://` URLs, both for `KOPS_BASE_URL` (nodeup download) and `spec.assets.fileRepository`. Nodes authenticate with their instance credentials; see the [asset repository documentation](https://kops.sigs.k8s.io/operations/asset-repository/) for the required permissions. The AWS nodeup download requires node images with curl 8.0 or newer.
+
 # Breaking changes
 
 * Support for AWS Classic Load Balancer (CLB) for the API has been removed. Clusters with `spec.api.loadBalancer.class: Classic` (or with no explicit `class`, which previously defaulted to Classic) fail validation, and the long-deprecated `kops create cluster --api-loadbalancer-class` flag has been removed. Existing clusters using a CLB must migrate to a Network Load Balancer (NLB) using kOps 1.36 or earlier before upgrading to kOps 1.37, following the [CLB to NLB migration guide](https://github.com/kubernetes/kops/blob/master/permalinks/acm_nlb.md). Attaching instance groups to externally-managed Classic Load Balancers via `spec.externalLoadBalancers[].loadBalancerName` remains supported.

hack/dev-build-aws.sh

@@ -14,10 +14,10 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-# This is a convenience script for developing kOps on GCE.
-# It builds the code, including nodeup, and uploads to a custom GCS bucket.
+# This is a convenience script for developing kOps on AWS.
+# It builds the code, including nodeup, and uploads to a custom S3 bucket.
 # It also sets KOPS_STATE_STORE and KOPS_BASE_URL to project-isolated values.
-# To use, source the script.  For example `. hack/dev-build-gce.sh` (note the initial `.`)
+# To use, source the script. For example, `. hack/dev-build-aws.sh` (note the initial `.`).
 
 # Can't use set -e in a script we want to source
 #set -e
@@ -37,9 +37,11 @@ aws s3 ls "${UPLOAD_DEST}" || aws s3 mb "${UPLOAD_DEST}" || return
 make kops-install dev-upload-linux-${KOPS_ARCH} || return
 
 # Set KOPS_BASE_URL
-(tools/get_version.sh | grep VERSION | awk '{print $2}') || return
-KOPS_VERSION=$(tools/get_version.sh | grep VERSION | awk '{print $2}')
-export KOPS_BASE_URL=https://${S3_BUCKET_NAME}.s3.amazonaws.com/kops/${KOPS_VERSION}/
+KOPS_VERSION=$(tools/get_version.sh | grep VERSION | awk '{print $2}') || return
+echo "${KOPS_VERSION}"
+# The s3:// form lets nodes use their instance profiles; https:// requires public read access.
+# Grant the cluster instance profiles s3:GetObject on the bucket and use node images with curl >= 8.0.
+export KOPS_BASE_URL=s3://${S3_BUCKET_NAME}/kops/${KOPS_VERSION}/
 
 # Create the state-store bucket if it doesn't exist
 KOPS_STATE_STORE="s3://kops-state-${ACCOUNT_ID}-${USER}"

pkg/apis/kops/validation/aws.go

@@ -17,6 +17,7 @@ limitations under the License.
 package validation
 
 import (
+	"context"
 	"fmt"
 	"net"
 	"strconv"
@@ -63,13 +64,38 @@ func awsValidateCluster(c *kops.Cluster, strict bool) field.ErrorList {
 
 	allErrs = append(allErrs, awsValidateUseIPBasedNodeNames(c)...)
 
+	allErrs = append(allErrs, awsValidateS3FileRepositoryPartition(c)...)
+
 	if c.Spec.Authentication != nil && c.Spec.Authentication.AWS != nil {
 		allErrs = append(allErrs, awsValidateIAMAuthenticator(field.NewPath("spec", "authentication", "aws"), c.Spec.Authentication.AWS)...)
 	}
 
 	return allErrs
 }
 
+func awsValidateS3FileRepositoryPartition(cluster *kops.Cluster) field.ErrorList {
+	assets := cluster.Spec.Assets
+	if assets == nil || assets.FileRepository == nil || !strings.HasPrefix(*assets.FileRepository, "s3://") {
+		return nil
+	}
+
+	region, err := awsup.FindRegion(cluster)
+	if err != nil {
+		// Subnet validation reports invalid or missing zones separately.
+		return nil
+	}
+
+	fldPath := field.NewPath("spec", "assets", "fileRepository")
+	supported, err := awsup.SupportsS3BootstrapEndpoint(context.TODO(), region)
+	if err != nil {
+		return field.ErrorList{field.Invalid(fldPath, *assets.FileRepository, err.Error())}
+	}
+	if !supported {
+		return field.ErrorList{field.Forbidden(fldPath, fmt.Sprintf("s3:// fileRepository is not supported in AWS region %q", region))}
+	}
+	return nil
+}
+
 func awsValidateEBSCSIDriver(cluster *kops.Cluster) (allErrs field.ErrorList) {
 	c := cluster.Spec
 

pkg/apis/kops/validation/aws_test.go

@@ -923,6 +923,59 @@ func TestAWSAdditionalRoutes(t *testing.T) {
 	}
 }
 
+func TestAWSValidateS3FileRepository(t *testing.T) {
+	for _, tc := range []struct {
+		name           string
+		region         string
+		fileRepository string
+		expectedErrors []string
+	}{
+		{
+			name:           "commercial partition",
+			region:         "us-east-1",
+			fileRepository: "s3://example-k8s-assets/kops",
+		},
+		{
+			name:           "GovCloud partition",
+			region:         "us-gov-west-1",
+			fileRepository: "s3://example-k8s-assets/kops",
+		},
+		{
+			name:           "China partition",
+			region:         "cn-north-1",
+			fileRepository: "s3://example-k8s-assets/kops",
+			expectedErrors: []string{"Forbidden::spec.assets.fileRepository"},
+		},
+		{
+			name:           "ISO partition",
+			region:         "us-iso-east-1",
+			fileRepository: "s3://example-k8s-assets/kops",
+			expectedErrors: []string{"Forbidden::spec.assets.fileRepository"},
+		},
+		{
+			name:           "HTTPS repository in ISO partition",
+			region:         "us-iso-east-1",
+			fileRepository: "https://example.com/kops",
+		},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			fileRepository := tc.fileRepository
+			cluster := &kops.Cluster{
+				Spec: kops.ClusterSpec{
+					CloudProvider: kops.CloudProviderSpec{AWS: &kops.AWSSpec{}},
+					Assets:        &kops.AssetsSpec{FileRepository: &fileRepository},
+					Networking: kops.NetworkingSpec{
+						Subnets: []kops.ClusterSubnetSpec{{Name: "subnet", Zone: tc.region + "a"}},
+					},
+				},
+			}
+
+			errs := awsValidateCluster(cluster, false)
+			testErrors(t, tc.name, errs, tc.expectedErrors)
+		})
+	}
+}
+
 func TestAWSValidateNLBSecurityGroupMode(t *testing.T) {
 	grid := []struct {
 		Input          kops.ClusterSpec

pkg/apis/kops/validation/validation.go

@@ -801,8 +801,13 @@ func validateFileRepository(s string, fieldPath *field.Path, cloudProvider kops.
 		if cloudProvider != kops.CloudProviderGCE {
 			allErrs = append(allErrs, field.Invalid(fieldPath, s, fmt.Sprintf("gs:// fileRepository is only supported on GCE, but the cloud provider is %q", cloudProvider)))
 		}
+	case "s3":
+		// Only AWS instances can authenticate to S3 with their instance profile.
+		if cloudProvider != kops.CloudProviderAWS {
+			allErrs = append(allErrs, field.Invalid(fieldPath, s, fmt.Sprintf("s3:// fileRepository is only supported on AWS, but the cloud provider is %q", cloudProvider)))
+		}
 	default:
-		allErrs = append(allErrs, field.Invalid(fieldPath, s, "fileRepository must be an http:// or https:// URL"))
+		allErrs = append(allErrs, field.Invalid(fieldPath, s, "fileRepository must be an http://, https://, gs://, or s3:// URL"))
 	}
 	if u.Host == "" {
 		allErrs = append(allErrs, field.Invalid(fieldPath, s, "fileRepository must include a host"))

pkg/apis/kops/validation/validation_test.go

@@ -2292,6 +2292,15 @@ func TestValidateFileRepository(t *testing.T) {
 		{
 			Input: "http://example.com/files",
 		},
+		{
+			Input:         "s3://example-k8s-assets/kops",
+			CloudProvider: kops.CloudProviderAWS,
+		},
+		{
+			Input:          "s3://example-k8s-assets/kops",
+			CloudProvider:  kops.CloudProviderGCE,
+			ExpectedErrors: []string{"Invalid value::spec.assets.fileRepository"},
+		},
 		{
 			Input:          "s3://example-k8s-assets/kops",
 			ExpectedErrors: []string{"Invalid value::spec.assets.fileRepository"},

pkg/assets/assetcopy/copyfile.go

@@ -176,11 +176,11 @@ func writeFile(ctx context.Context, cluster *kops.Cluster, p vfs.Path, data []by
 	return nil
 }
 
-// buildVFSPath task a recognizable https url and transforms that URL into the equivalent url with the object
-// store prefix.
+// buildVFSPath returns local paths and memfs://, file://, gs://, and s3:// URLs unchanged.
+// It converts recognized S3 or GCS HTTPS URLs to their native VFS form.
 func buildVFSPath(target string) (string, error) {
 	if !strings.Contains(target, "://") || strings.HasPrefix(target, "memfs://") || strings.HasPrefix(target, "file://") ||
-		strings.HasPrefix(target, "gs://") {
+		strings.HasPrefix(target, "gs://") || strings.HasPrefix(target, "s3://") {
 		return target, nil
 	}
 
@@ -207,7 +207,7 @@ func buildVFSPath(target string) (string, error) {
 	if vfsPath == "" {
 		klog.Errorf("Unable to determine VFS path from supplied URL: %s", target)
 		klog.Errorf("S3, Google Cloud Storage, and File Paths are supported.")
-		klog.Errorf("For S3, please make sure that the supplied file repository URL adhere to S3 naming conventions, https://docs.aws.amazon.com/general/latest/gr/rande.html#s3_region.")
+		klog.Errorf("For S3, please make sure that the supplied file repository URL starts with s3:// or adheres to S3 naming conventions.")
 		klog.Errorf("For GCS, please make sure that the supplied file repository URL starts with gs:// or https://storage.googleapis.com/")
 		if err != nil { // print the S3 error for more details
 			return "", fmt.Errorf("Error Details: %v", err)

pkg/assets/assetcopy/copyfile_test.go

@@ -51,6 +51,11 @@ func Test_BuildVFSPath(t *testing.T) {
 			"s3://k8s-for-greeks-kops/kubernetes-release/release/v1.7.2/bin/linux/amd64/kubectl",
 			true,
 		},
+		{
+			"s3://k8s-for-greeks-kops/kubernetes-release/release/v1.7.2/bin/linux/amd64/kubectl",
+			"s3://k8s-for-greeks-kops/kubernetes-release/release/v1.7.2/bin/linux/amd64/kubectl",
+			true,
+		},
 		{
 			"https://foo/k8s-for-greeks-kops/kubernetes-release/release/v1.7.2/bin/linux/amd64/kubectl",
 			"",

pkg/assets/builder.go

@@ -442,6 +442,8 @@ func (a *AssetBuilder) remapURL(canonicalURL *url.URL) (*url.URL, error) {
 	}
 
 	fileRepo.Path = path.Join(fileRepo.Path, canonicalURL.Path)
+	// Escape commas, which are legal in a path but separate locations in CompactString.
+	fileRepo.RawPath = strings.ReplaceAll(fileRepo.EscapedPath(), ",", "%2C")
 
 	return fileRepo, nil
 }