Repository: goreleaser/goreleaser
Pull request #2057 (2021-02-12 14:23:51)

fix: makes username and password optional for http uploads
This PR resolves https://github.com/goreleaser/goreleaser/issues/2008 by making username and password optional for http uploads

As per the documentation (https://goreleaser.com/customization/upload/), username and password are required only if the server requires it. However, it was being imposed in all the cases.

vercel[bot]: @dpaks is attempting to deploy a commit to the **Goreleaser** Team on [Vercel](https://vercel.com).

A member of the Team first needs to [authorize it](https://vercel.com/git/authorize?team=Goreleaser&type=github&job=%7B%22headInfo%22%3A%7B%22sha%22%3A%22ce869690f3aee5a4481c5c8f9ccae926fc04f24a%22%7D%2C%22id%22%3A%22QmWYgWD6cFmJaLpU5Lc4j8UDmCf5ueA3spSHNMSZC8GokL%22%2C%22org%22%3A%22goreleaser%22%2C%22prId%22%3A2057%2C%22repo%22%3A%22goreleaser%22%7D).


codecov[bot]: # [Codecov](https://codecov.io/gh/goreleaser/goreleaser/pull/2057?src=pr&el=h1) Report
> Merging [#2057](https://codecov.io/gh/goreleaser/goreleaser/pull/2057?src=pr&el=desc) (ce86969) into [master](https://codecov.io/gh/goreleaser/goreleaser/commit/f6f448d84e2f93bbbab35e3dd7bf253340610a47?el=desc) (f6f448d) will **increase** coverage by `0.07%`.
> The diff coverage is `100.00%`.

[![Impacted file tree graph](https://codecov.io/gh/goreleaser/goreleaser/pull/2057/graphs/tree.svg?width=650&height=150&src=pr&token=3bJvv6b7hN)](https://codecov.io/gh/goreleaser/goreleaser/pull/2057?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master    #2057      +/-   ##
==========================================
+ Coverage   82.88%   82.95%   +0.07%     
==========================================
  Files          74       74              
  Lines        3979     3972       -7     
==========================================
- Hits         3298     3295       -3     
+ Misses        555      553       -2     
+ Partials      126      124       -2     
```


| [Impacted Files](https://codecov.io/gh/goreleaser/goreleaser/pull/2057?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [internal/http/http.go](https://codecov.io/gh/goreleaser/goreleaser/pull/2057/diff?src=pr&el=tree#diff-aW50ZXJuYWwvaHR0cC9odHRwLmdv) | `91.84% <100.00%> (+1.79%)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/goreleaser/goreleaser/pull/2057?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/goreleaser/goreleaser/pull/2057?src=pr&el=footer). Last update [f6f448d...c42f3cc](https://codecov.io/gh/goreleaser/goreleaser/pull/2057?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

caarlos0: Thanks, LGTM!

Can we update docs too? 🙏
dpaks: @caarlos0 thank you! I've updated the docs too. Hope I didn't screw it :smiley: 

Patch:
diff --git a/internal/http/http.go b/internal/http/http.go
index ce1819ee..f76155de 100644
--- a/internal/http/http.go
+++ b/internal/http/http.go
@@ -96,12 +96,16 @@ func CheckConfig(ctx *context.Context, upload *config.Upload, kind string) error
 		return misconfigured(kind, upload, "mode must be 'binary' or 'archive'")
 	}
 
-	if _, err := getUsername(ctx, upload, kind); err != nil {
-		return err
+	username := getUsername(ctx, upload, kind)
+	password := getPassword(ctx, upload, kind)
+	passwordEnv := fmt.Sprintf("%s_%s_SECRET", strings.ToUpper(kind), strings.ToUpper(upload.Name))
+
+	if password != "" && username == "" {
+		return misconfigured(kind, upload, fmt.Sprintf("'username' is required when '%s' environment variable is set", passwordEnv))
 	}
 
-	if _, err := getPassword(ctx, upload, kind); err != nil {
-		return err
+	if username != "" && password == "" {
+		return misconfigured(kind, upload, fmt.Sprintf("environment variable '%s' is required when 'username' is set", passwordEnv))
 	}
 
 	if upload.TrustedCerts != "" && !x509.NewCertPool().AppendCertsFromPEM([]byte(upload.TrustedCerts)) {
@@ -111,25 +115,20 @@ func CheckConfig(ctx *context.Context, upload *config.Upload, kind string) error
 	return nil
 }
 
-func getUsername(ctx *context.Context, upload *config.Upload, kind string) (string, error) {
+// username is optional
+func getUsername(ctx *context.Context, upload *config.Upload, kind string) string {
 	if upload.Username != "" {
-		return upload.Username, nil
+		return upload.Username
 	}
+
 	var key = fmt.Sprintf("%s_%s_USERNAME", strings.ToUpper(kind), strings.ToUpper(upload.Name))
-	user, ok := ctx.Env[key]
-	if !ok {
-		return "", misconfigured(kind, upload, fmt.Sprintf("missing username or %s environment variable", key))
-	}
-	return user, nil
+	return ctx.Env[key]
 }
 
-func getPassword(ctx *context.Context, upload *config.Upload, kind string) (string, error) {
+// password is optional
+func getPassword(ctx *context.Context, upload *config.Upload, kind string) string {
 	var key = fmt.Sprintf("%s_%s_SECRET", strings.ToUpper(kind), strings.ToUpper(upload.Name))
-	pwd, ok := ctx.Env[key]
-	if !ok {
-		return "", misconfigured(kind, upload, fmt.Sprintf("missing %s environment variable", key))
-	}
-	return pwd, nil
+	return ctx.Env[key]
 }
 
 func misconfigured(kind string, upload *config.Upload, reason string) error {
@@ -204,15 +203,10 @@ func uploadWithFilter(ctx *context.Context, upload *config.Upload, filter artifa
 
 // uploadAsset uploads file to target and logs all actions.
 func uploadAsset(ctx *context.Context, upload *config.Upload, artifact *artifact.Artifact, kind string, check ResponseChecker) error {
-	username, err := getUsername(ctx, upload, kind)
-	if err != nil {
-		return err
-	}
-
-	secret, err := getPassword(ctx, upload, kind)
-	if err != nil {
-		return err
-	}
+	// username and secret are optional since the server may not support/need
+	// basic authentication always
+	username := getUsername(ctx, upload, kind)
+	secret := getPassword(ctx, upload, kind)
 
 	// Generate the target url
 	targetURL, err := resolveTargetTemplate(ctx, upload, artifact)
@@ -268,7 +262,6 @@ func uploadAsset(ctx *context.Context, upload *config.Upload, artifact *artifact
 		msg := fmt.Sprintf("%s: upload failed", kind)
 		log.WithError(err).WithFields(log.Fields{
 			"instance": upload.Name,
-			"username": username,
 		}).Error(msg)
 		return fmt.Errorf("%s: %w", msg, err)
 	}
@@ -301,7 +294,10 @@ func newUploadRequest(ctx *context.Context, method, target, username, secret str
 		return nil, err
 	}
 	req.ContentLength = a.Size
-	req.SetBasicAuth(username, secret)
+
+	if username != "" && secret != "" {
+		req.SetBasicAuth(username, secret)
+	}
 
 	for k, v := range headers {
 		req.Header.Add(k, v)
diff --git a/www/docs/customization/upload.md b/www/docs/customization/upload.md
index 93f97618..6bc9ae95 100644
--- a/www/docs/customization/upload.md
+++ b/www/docs/customization/upload.md
@@ -11,19 +11,18 @@ You can declare multiple `uploads` instances. All binaries generated by your
 `builds` section will be pushed to each configured upload.
 
 If you have only one `uploads` instance, the configuration is as easy as adding
-the upload target and a username to your `.goreleaser.yml` file:
+the upload target and a name to your `.goreleaser.yml` file:
 
 ```yaml
 uploads:
   - name: production
     target: http://some.server/some/path/example-repo-local/{{ .ProjectName }}/{{ .Version }}/
-    username: goreleaser
 ```
 
 Prerequisites:
 
 - An HTTP server accepting HTTP requests
-- A user + password with grants to upload an artifact using HTTP requests (if the server requires it)
+- A user + password with grants to upload an artifact using HTTP requests for basic authentication (only if the server requires it)
 
 ### Target
 
@@ -60,7 +59,7 @@ generated by `nfpm` and the like.
 Your configured username needs to be valid against your HTTP server.
 
 You can have the username set in the configuration file as shown above
-or you can have it read from and environment variable.
+or you can have it read from an environment variable.
 The configured name of your HTTP server will be used to build the environment
 variable name.
 This way we support auth for multiple instances.
@@ -75,9 +74,11 @@ The name will be transformed to uppercase.
 If a configured username is found in the configuration file, then the
 environment variable is not used at all.
 
+This field is optional and is used only for basic http authentication.
+
 ### Password
 
-The password will be stored in a environment variable.
+The password will be stored in an environment variable.
 The configured name of your HTTP server will be used.
 This way we support auth for multiple instances.
 This also means that the `name` per configured instance needs to be unique
@@ -88,6 +89,8 @@ If your instance is named `production`, you need to store the secret in the
 environment variable `UPLOAD_PRODUCTION_SECRET`.
 The name will be transformed to uppercase.
 
+This field is optional and is used only for basic http authentication.
+
 ### Server authentication
 
 You can authenticate your TLS server adding a trusted X.509 certificate chain
@@ -153,7 +156,7 @@ uploads:
     # target: https://some.server/some/path/example-repo-local/{{ .ArtifactName }};deb.distribution=xenial
     custom_artifact_name: true
 
-    # User that will be used for the deployment
+    # An optional username that will be used for the deployment for basic authn
     username: deployuser
 
     # An optional header you can use to tell GoReleaser to pass the artifact's


Test patch:
diff --git a/internal/http/http_test.go b/internal/http/http_test.go
index ff1f0cf3..b2d938ff 100644
--- a/internal/http/http_test.go
+++ b/internal/http/http_test.go
@@ -93,6 +93,8 @@ func TestCheckConfig(t *testing.T) {
 		{"secret missing", args{ctx, &config.Upload{Name: "b", Target: "http://blabla", Username: "pepe", Mode: ModeArchive}, "test"}, true},
 		{"target missing", args{ctx, &config.Upload{Name: "a", Username: "pepe", Mode: ModeArchive}, "test"}, true},
 		{"name missing", args{ctx, &config.Upload{Target: "http://blabla", Username: "pepe", Mode: ModeArchive}, "test"}, true},
+		{"username missing", args{ctx, &config.Upload{Name: "a", Target: "http://blabla", Mode: ModeArchive}, "test"}, true},
+		{"username present", args{ctx, &config.Upload{Name: "a", Target: "http://blabla", Username: "pepe", Mode: ModeArchive}, "test"}, false},
 		{"mode missing", args{ctx, &config.Upload{Name: "a", Target: "http://blabla", Username: "pepe"}, "test"}, true},
 		{"mode invalid", args{ctx, &config.Upload{Name: "a", Target: "http://blabla", Username: "pepe", Mode: "blabla"}, "test"}, true},
 		{"cert invalid", args{ctx, &config.Upload{Name: "a", Target: "http://blabla", Username: "pepe", Mode: ModeBinary, TrustedCerts: "bad cert!"}, "test"}, true},
@@ -104,6 +106,24 @@ func TestCheckConfig(t *testing.T) {
 			}
 		})
 	}
+
+	delete(ctx.Env, "TEST_A_SECRET")
+
+	tests = []struct {
+		name    string
+		args    args
+		wantErr bool
+	}{
+		{"username missing", args{ctx, &config.Upload{Name: "a", Target: "http://blabla", Mode: ModeArchive}, "test"}, false},
+		{"username present", args{ctx, &config.Upload{Name: "a", Target: "http://blabla", Username: "pepe", Mode: ModeArchive}, "test"}, true},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			if err := CheckConfig(tt.args.ctx, tt.args.upload, tt.args.kind); (err != nil) != tt.wantErr {
+				t.Errorf("CheckConfig() error = %v, wantErr %v", err, tt.wantErr)
+			}
+		})
+	}
 }
 
 type check struct {