pkg/storage/s3: use force path style in favour of virtual hosted style

the latest upgrade of our openshift/docker-distribution fork brought in
a patch introducing `forcepathstyle` configuration, which was mistakenly
merged the openshift only `virtualhostedstyle` configuration.

to workaround the issue we simply drop the use of `virtualhostedstyle`
in favour of `forcepathstyle`.

we will eventually remove our custom `virtualhostedstyle` path from our
docker-distribution fork, but using
`.spec.storage.s3.virtualHostedStyle` should work as expected
regardless.

Author: flavianmissi

pkg/resource/podtemplatespec_test.go

@@ -521,7 +521,7 @@ func TestMakePodTemplateSpecS3CloudFront(t *testing.T) {
 		"REGISTRY_STORAGE_S3_BUCKET":                {Value: "bucket"},
 		"REGISTRY_STORAGE_S3_REGION":                {Value: "region"},
 		"REGISTRY_STORAGE_S3_ENCRYPT":               {Value: "true"},
-		"REGISTRY_STORAGE_S3_VIRTUALHOSTEDSTYLE":    {Value: "true"},
+		"REGISTRY_STORAGE_S3_FORCEPATHSTYLE":        {Value: "false"},
 		"REGISTRY_STORAGE_S3_USEDUALSTACK":          {Value: "true"},
 		"REGISTRY_STORAGE_S3_CREDENTIALSCONFIGPATH": {Value: "/var/run/secrets/cloud/credentials"},
 		"REGISTRY_MIDDLEWARE_STORAGE": {Value: `- name: cloudfront

pkg/storage/ibmcos/ibmcos.go

@@ -88,7 +88,7 @@ func (d *driver) ConfigEnv() (envs envvar.List, err error) {
 		envvar.EnvVar{Name: "REGISTRY_STORAGE_S3_REGION", Value: d.Config.Location},
 		envvar.EnvVar{Name: "REGISTRY_STORAGE_S3_REGIONENDPOINT", Value: fmt.Sprintf("s3.%s.cloud-object-storage.appdomain.cloud", d.Config.Location)},
 		envvar.EnvVar{Name: "REGISTRY_STORAGE_S3_ENCRYPT", Value: false},
-		envvar.EnvVar{Name: "REGISTRY_STORAGE_S3_VIRTUALHOSTEDSTYLE", Value: false},
+		envvar.EnvVar{Name: "REGISTRY_STORAGE_S3_FORCEPATHSTYLE", Value: true},
 		envvar.EnvVar{Name: "REGISTRY_STORAGE_S3_USEDUALSTACK", Value: false},
 		envvar.EnvVar{Name: "REGISTRY_STORAGE_S3_CREDENTIALSCONFIGPATH", Value: filepath.Join(imageRegistrySecretMountpoint, imageRegistrySecretDataKey)},
 	)

pkg/storage/ibmcos/ibmcos_test.go

@@ -60,7 +60,7 @@ func TestConfigEnv(t *testing.T) {
 		"REGISTRY_STORAGE_S3_REGION":                "us-east",
 		"REGISTRY_STORAGE_S3_REGIONENDPOINT":        "s3.us-east.cloud-object-storage.appdomain.cloud",
 		"REGISTRY_STORAGE_S3_ENCRYPT":               false,
-		"REGISTRY_STORAGE_S3_VIRTUALHOSTEDSTYLE":    false,
+		"REGISTRY_STORAGE_S3_FORCEPATHSTYLE":        true,
 		"REGISTRY_STORAGE_S3_USEDUALSTACK":          false,
 		"REGISTRY_STORAGE_S3_CREDENTIALSCONFIGPATH": filepath.Join(imageRegistrySecretMountpoint, imageRegistrySecretDataKey),
 	}

pkg/storage/s3/s3.go

@@ -409,12 +409,31 @@ func (d *driver) ConfigEnv() (envs envvar.List, err error) {
 		envs = append(envs, envvar.EnvVar{Name: "REGISTRY_STORAGE_S3_KEYID", Value: d.Config.KeyID})
 	}
 
+	// virtualHostedStyle tells the registry to use urls in the form of
+	// bucket-name.s3-endpoint.etc.
+	// the forcePathStyle setting was introduced to control the same
+	// behaviour, but it's named after the opposite setting (path style
+	// instead of virtual hosted style):
+	// s3-endpoint.etc/bucket-name.
+	// the PR that introduced virtual hosted style to upstream distribution
+	// was never merged: https://github.com/distribution/distribution/pull/3131/files
+	// and it's only present in our fork:
+	//  * https://github.com/openshift/docker-distribution/commit/e33e2357eb705f2ed7481e3510fceedb01a95bb6
+	//  * https://github.com/openshift/docker-distribution/commit/063574e3222f00556ec5113dddca9a0ac28ed4cb
+	// the upstream introduction of the force path style config happened in:
+	//  * https://github.com/distribution/distribution/commit/15de9e21bad774b24e48a46d9238a5714e7ceb6c
+	// and it's also present in our fork.
+	// TODO: drop commits from openshift/docker-distribution during next rebase:
+	//  * https://github.com/openshift/docker-distribution/commit/e33e2357eb705f2ed7481e3510fceedb01a95bb6
+	//  * https://github.com/openshift/docker-distribution/commit/063574e3222f00556ec5113dddca9a0ac28ed4cb
+	// Jira tracker: https://issues.redhat.com/browse/IR-470
+	forcePathStyle := !d.Config.VirtualHostedStyle
 	envs = append(envs,
 		envvar.EnvVar{Name: "REGISTRY_STORAGE", Value: "s3"},
 		envvar.EnvVar{Name: "REGISTRY_STORAGE_S3_BUCKET", Value: d.Config.Bucket},
 		envvar.EnvVar{Name: "REGISTRY_STORAGE_S3_REGION", Value: d.Config.Region},
 		envvar.EnvVar{Name: "REGISTRY_STORAGE_S3_ENCRYPT", Value: d.Config.Encrypt},
-		envvar.EnvVar{Name: "REGISTRY_STORAGE_S3_VIRTUALHOSTEDSTYLE", Value: d.Config.VirtualHostedStyle},
+		envvar.EnvVar{Name: "REGISTRY_STORAGE_S3_FORCEPATHSTYLE", Value: forcePathStyle},
 		envvar.EnvVar{Name: "REGISTRY_STORAGE_S3_CREDENTIALSCONFIGPATH", Value: filepath.Join(imageRegistrySecretMountpoint, imageRegistrySecretDataKey)},
 	)
 

pkg/storage/s3/s3_test.go

@@ -267,7 +267,7 @@ func TestConfigEnv(t *testing.T) {
 		"REGISTRY_STORAGE":                          "s3",
 		"REGISTRY_STORAGE_S3_REGION":                "us-east-1",
 		"REGISTRY_STORAGE_S3_USEDUALSTACK":          true,
-		"REGISTRY_STORAGE_S3_VIRTUALHOSTEDSTYLE":    false,
+		"REGISTRY_STORAGE_S3_FORCEPATHSTYLE":        true,
 		"REGISTRY_STORAGE_S3_CREDENTIALSCONFIGPATH": filepath.Join(imageRegistrySecretMountpoint, imageRegistrySecretDataKey),
 	}
 	for key, value := range expectedVars {
@@ -331,7 +331,7 @@ func TestServiceEndpointCanBeOverwritten(t *testing.T) {
 		"REGISTRY_STORAGE":                          "s3",
 		"REGISTRY_STORAGE_S3_REGION":                "us-west-1",
 		"REGISTRY_STORAGE_S3_USEDUALSTACK":          true,
-		"REGISTRY_STORAGE_S3_VIRTUALHOSTEDSTYLE":    false,
+		"REGISTRY_STORAGE_S3_FORCEPATHSTYLE":        true,
 		"REGISTRY_STORAGE_S3_CREDENTIALSCONFIGPATH": filepath.Join(imageRegistrySecretMountpoint, imageRegistrySecretDataKey),
 	}
 	for key, value := range expectedVars {