Repository: prometheus/common
Pull request #297 (2021-05-05 20:17:29)

Adds json struct tags to mirror yaml ones
This PR includes json struct tags to mirror the yaml ones in config objects. This allows downstream projects to consume these with a consistent key structure regardless of format. I'm hoping this isn't too controversial :).

For context: I'd like to consolidate these so Grafana can more easily present a JSON API over the Alertmanager structs and seamlessly transition json<>yaml. The lack of tag synchronization combined with the `<secret>` redaction in marshaling code makes this a bit more difficult than I'd like.

Interestingly, this has already been done with [Alertmanager](https://github.com/prometheus/alertmanager/blob/master/config/config.go#L267-L277) but not with [Prometheus](https://github.com/prometheus/prometheus/blob/main/config/config.go#L187-L195).

Please let me know if this is reasonable, and if so, if you'd like me to add some marshaling testware.
/cc @roidelapluie 


Patch:
diff --git a/config/config.go b/config/config.go
index d8a7351..fffda4a 100644
--- a/config/config.go
+++ b/config/config.go
@@ -16,7 +16,12 @@
 
 package config
 
-import "path/filepath"
+import (
+	"encoding/json"
+	"path/filepath"
+)
+
+const secretToken = "<secret>"
 
 // Secret special type for storing secrets.
 type Secret string
@@ -24,7 +29,7 @@ type Secret string
 // MarshalYAML implements the yaml.Marshaler interface for Secrets.
 func (s Secret) MarshalYAML() (interface{}, error) {
 	if s != "" {
-		return "<secret>", nil
+		return secretToken, nil
 	}
 	return nil, nil
 }
@@ -35,6 +40,14 @@ func (s *Secret) UnmarshalYAML(unmarshal func(interface{}) error) error {
 	return unmarshal((*plain)(s))
 }
 
+// MarshalJSON implements the json.Marshaler interface for Secret.
+func (s Secret) MarshalJSON() ([]byte, error) {
+	if len(s) == 0 {
+		return json.Marshal("")
+	}
+	return json.Marshal(secretToken)
+}
+
 // DirectorySetter is a config type that contains file paths that may
 // be relative to the file containing the config.
 type DirectorySetter interface {
diff --git a/config/http_config.go b/config/http_config.go
index b98ff7c..d19221a 100644
--- a/config/http_config.go
+++ b/config/http_config.go
@@ -21,6 +21,7 @@ import (
 	"crypto/sha256"
 	"crypto/tls"
 	"crypto/x509"
+	"encoding/json"
 	"fmt"
 	"io/ioutil"
 	"net"
@@ -54,9 +55,9 @@ type closeIdler interface {
 
 // BasicAuth contains basic HTTP authentication credentials.
 type BasicAuth struct {
-	Username     string `yaml:"username"`
-	Password     Secret `yaml:"password,omitempty"`
-	PasswordFile string `yaml:"password_file,omitempty"`
+	Username     string `yaml:"username" json:"username"`
+	Password     Secret `yaml:"password,omitempty" json:"password,omitempty"`
+	PasswordFile string `yaml:"password_file,omitempty" json:"password_file,omitempty"`
 }
 
 // SetDirectory joins any relative file paths with dir.
@@ -69,9 +70,9 @@ func (a *BasicAuth) SetDirectory(dir string) {
 
 // Authorization contains HTTP authorization credentials.
 type Authorization struct {
-	Type            string `yaml:"type,omitempty"`
-	Credentials     Secret `yaml:"credentials,omitempty"`
-	CredentialsFile string `yaml:"credentials_file,omitempty"`
+	Type            string `yaml:"type,omitempty" json:"type,omitempty"`
+	Credentials     Secret `yaml:"credentials,omitempty" json:"credentials,omitempty"`
+	CredentialsFile string `yaml:"credentials_file,omitempty" json:"credentials_file,omitempty"`
 }
 
 // SetDirectory joins any relative file paths with dir.
@@ -110,14 +111,36 @@ func (u URL) MarshalYAML() (interface{}, error) {
 	return nil, nil
 }
 
+// UnmarshalJSON implements the json.Marshaler interface for URL.
+func (u *URL) UnmarshalJSON(data []byte) error {
+	var s string
+	if err := json.Unmarshal(data, &s); err != nil {
+		return err
+	}
+	urlp, err := url.Parse(s)
+	if err != nil {
+		return err
+	}
+	u.URL = urlp
+	return nil
+}
+
+// MarshalJSON implements the json.Marshaler interface for URL.
+func (u URL) MarshalJSON() ([]byte, error) {
+	if u.URL != nil {
+		return json.Marshal(u.URL.String())
+	}
+	return nil, nil
+}
+
 // OAuth2 is the oauth2 client configuration.
 type OAuth2 struct {
-	ClientID         string            `yaml:"client_id"`
-	ClientSecret     Secret            `yaml:"client_secret"`
-	ClientSecretFile string            `yaml:"client_secret_file"`
-	Scopes           []string          `yaml:"scopes,omitempty"`
-	TokenURL         string            `yaml:"token_url"`
-	EndpointParams   map[string]string `yaml:"endpoint_params,omitempty"`
+	ClientID         string            `yaml:"client_id" json:"client_id"`
+	ClientSecret     Secret            `yaml:"client_secret" json:"client_secret"`
+	ClientSecretFile string            `yaml:"client_secret_file" json:"client_secret_file"`
+	Scopes           []string          `yaml:"scopes,omitempty" json:"scopes,omitempty"`
+	TokenURL         string            `yaml:"token_url" json:"token_url"`
+	EndpointParams   map[string]string `yaml:"endpoint_params,omitempty" json:"endpoint_params,omitempty"`
 }
 
 // SetDirectory joins any relative file paths with dir.
@@ -131,25 +154,25 @@ func (a *OAuth2) SetDirectory(dir string) {
 // HTTPClientConfig configures an HTTP client.
 type HTTPClientConfig struct {
 	// The HTTP basic authentication credentials for the targets.
-	BasicAuth *BasicAuth `yaml:"basic_auth,omitempty"`
+	BasicAuth *BasicAuth `yaml:"basic_auth,omitempty" json:"basic_auth,omitempty"`
 	// The HTTP authorization credentials for the targets.
-	Authorization *Authorization `yaml:"authorization,omitempty"`
+	Authorization *Authorization `yaml:"authorization,omitempty" json:"authorization,omitempty"`
 	// The OAuth2 client credentials used to fetch a token for the targets.
-	OAuth2 *OAuth2 `yaml:"oauth2,omitempty"`
+	OAuth2 *OAuth2 `yaml:"oauth2,omitempty" json:"oauth2,omitempty"`
 	// The bearer token for the targets. Deprecated in favour of
 	// Authorization.Credentials.
-	BearerToken Secret `yaml:"bearer_token,omitempty"`
+	BearerToken Secret `yaml:"bearer_token,omitempty" json:"bearer_token,omitempty"`
 	// The bearer token file for the targets. Deprecated in favour of
 	// Authorization.CredentialsFile.
-	BearerTokenFile string `yaml:"bearer_token_file,omitempty"`
+	BearerTokenFile string `yaml:"bearer_token_file,omitempty" json:"bearer_token_file,omitempty"`
 	// HTTP proxy server to use to connect to the targets.
-	ProxyURL URL `yaml:"proxy_url,omitempty"`
+	ProxyURL URL `yaml:"proxy_url,omitempty" json:"proxy_url,omitempty"`
 	// TLSConfig to use to connect to the targets.
-	TLSConfig TLSConfig `yaml:"tls_config,omitempty"`
+	TLSConfig TLSConfig `yaml:"tls_config,omitempty" json:"tls_config,omitempty"`
 	// FollowRedirects specifies whether the client should follow HTTP 3xx redirects.
 	// The omitempty flag is not set, because it would be hidden from the
 	// marshalled configuration when set to false.
-	FollowRedirects bool `yaml:"follow_redirects"`
+	FollowRedirects bool `yaml:"follow_redirects" json:"follow_redirects"`
 }
 
 // SetDirectory joins any relative file paths with dir.
@@ -236,6 +259,16 @@ func (c *HTTPClientConfig) UnmarshalYAML(unmarshal func(interface{}) error) erro
 	return c.Validate()
 }
 
+// UnmarshalJSON implements the json.Marshaler interface for URL.
+func (c *HTTPClientConfig) UnmarshalJSON(data []byte) error {
+	type plain HTTPClientConfig
+	*c = DefaultHTTPClientConfig
+	if err := json.Unmarshal(data, (*plain)(c)); err != nil {
+		return err
+	}
+	return c.Validate()
+}
+
 // UnmarshalYAML implements the yaml.Unmarshaler interface.
 func (a *BasicAuth) UnmarshalYAML(unmarshal func(interface{}) error) error {
 	type plain BasicAuth
@@ -616,15 +649,15 @@ func NewTLSConfig(cfg *TLSConfig) (*tls.Config, error) {
 // TLSConfig configures the options for TLS connections.
 type TLSConfig struct {
 	// The CA cert to use for the targets.
-	CAFile string `yaml:"ca_file,omitempty"`
+	CAFile string `yaml:"ca_file,omitempty" json:"ca_file,omitempty"`
 	// The client cert file for the targets.
-	CertFile string `yaml:"cert_file,omitempty"`
+	CertFile string `yaml:"cert_file,omitempty" json:"cert_file,omitempty"`
 	// The client key file for the targets.
-	KeyFile string `yaml:"key_file,omitempty"`
+	KeyFile string `yaml:"key_file,omitempty" json:"key_file,omitempty"`
 	// Used to verify the hostname for the targets.
-	ServerName string `yaml:"server_name,omitempty"`
+	ServerName string `yaml:"server_name,omitempty" json:"server_name,omitempty"`
 	// Disable target certificate validation.
-	InsecureSkipVerify bool `yaml:"insecure_skip_verify"`
+	InsecureSkipVerify bool `yaml:"insecure_skip_verify" json:"insecure_skip_verify"`
 }
 
 // SetDirectory joins any relative file paths with dir.


Test patch:
diff --git a/config/config_test.go b/config/config_test.go
new file mode 100644
index 0000000..81c7855
--- /dev/null
+++ b/config/config_test.go
@@ -0,0 +1,55 @@
+// Copyright 2021 The Prometheus Authors
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+// +build go1.8
+
+package config
+
+import (
+	"encoding/json"
+	"testing"
+)
+
+func TestJSONMarshalSecret(t *testing.T) {
+	type tmp struct {
+		S Secret
+	}
+	for _, tc := range []struct {
+		desc     string
+		data     tmp
+		expected string
+	}{
+		{
+			desc: "inhabited",
+			// u003c -> "<"
+			// u003e -> ">"
+			data:     tmp{"test"},
+			expected: "{\"S\":\"\\u003csecret\\u003e\"}",
+		},
+		{
+			desc:     "empty",
+			data:     tmp{},
+			expected: "{\"S\":\"\"}",
+		},
+	} {
+		t.Run(tc.desc, func(t *testing.T) {
+			c, err := json.Marshal(tc.data)
+			if err != nil {
+				t.Fatal(err)
+			}
+			if tc.expected != string(c) {
+				t.Fatalf("Secret not marshaled correctly, got '%s'", string(c))
+			}
+		})
+	}
+}
diff --git a/config/http_config_test.go b/config/http_config_test.go
index aff3b8a..66fb3de 100644
--- a/config/http_config_test.go
+++ b/config/http_config_test.go
@@ -26,6 +26,7 @@ import (
 	"net"
 	"net/http"
 	"net/http/httptest"
+	"net/url"
 	"os"
 	"path/filepath"
 	"reflect"
@@ -1276,3 +1277,48 @@ endpoint_params:
 		t.Fatalf("Expected authorization header to be 'Bearer 12345', got '%s'", authorization)
 	}
 }
+
+func TestMarshalURL(t *testing.T) {
+	urlp, err := url.Parse("http://example.com/")
+	if err != nil {
+		t.Fatal(err)
+	}
+	u := &URL{urlp}
+
+	c, err := json.Marshal(u)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if string(c) != "\"http://example.com/\"" {
+		t.Fatalf("URL not properly marshaled in JSON got '%s'", string(c))
+	}
+
+	c, err = yaml.Marshal(u)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if string(c) != "http://example.com/\n" {
+		t.Fatalf("URL not properly marshaled in YAML got '%s'", string(c))
+	}
+}
+
+func TestUnmarshalURL(t *testing.T) {
+	b := []byte(`"http://example.com/a b"`)
+	var u URL
+
+	err := json.Unmarshal(b, &u)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if u.String() != "http://example.com/a%20b" {
+		t.Fatalf("URL not properly unmarshaled in JSON, got '%s'", u.String())
+	}
+
+	err = yaml.Unmarshal(b, &u)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if u.String() != "http://example.com/a%20b" {
+		t.Fatalf("URL not properly unmarshaled in YAML, got '%s'", u.String())
+	}
+}