synced/coder
1
0
Fork 0
mirror of https://github.com/coder/coder.git synced 2025-07-21 01:28:49 +00:00
Code Issues Packages Projects Releases Wiki Activity

chore: color value_source for deployment values (#9922)

Browse Source 
* chore: Return populated options vs a blank
* Strip secret values
This commit is contained in:
Steven Masley
2023-09-29 12:04:28 -05:00
committed by GitHub
parent 87ebe6c2c2
commit 92308bec3b
12 changed files with 410 additions and 34 deletions
Download Patch File Download Diff File Expand all files Collapse all files

138
cli/clibase/option.go
View File

@ -1,6 +1,8 @@
package clibase
import (
"bytes"
"encoding/json"
"os"
"strings"
@ -65,6 +67,20 @@ type Option struct {
ValueSource ValueSource `json:"value_source,omitempty"`
}
// optionNoMethods is just a wrapper around Option so we can defer to the
// default json.Unmarshaler behavior.
type optionNoMethods Option
func (o *Option) UnmarshalJSON(data []byte) error {
// If an option has no values, we have no idea how to unmarshal it.
// So just discard the json data.
if o.Value == nil {
o.Value = &DiscardValue
}
return json.Unmarshal(data, (*optionNoMethods)(o))
}
func (o Option) YAMLPath() string {
if o.YAML == "" {
return ""
@ -79,15 +95,101 @@ func (o Option) YAMLPath() string {
// OptionSet is a group of options that can be applied to a command.
type OptionSet []Option
// UnmarshalJSON implements json.Unmarshaler for OptionSets. Options have an
// interface Value type that cannot handle unmarshalling because the types cannot
// be inferred. Since it is a slice, instantiating the Options first does not
// help.
//
// However, we typically do instantiate the slice to have the correct types.
// So this unmarshaller will attempt to find the named option in the existing
// set, if it cannot, the value is discarded. If the option exists, the value
// is unmarshalled into the existing option, and replaces the existing option.
//
// The value is discarded if it's type cannot be inferred. This behavior just
// feels "safer", although it should never happen if the correct option set
// is passed in. The situation where this could occur is if a client and server
// are on different versions with different options.
func (optSet *OptionSet) UnmarshalJSON(data []byte) error {
dec := json.NewDecoder(bytes.NewBuffer(data))
// Should be a json array, so consume the starting open bracket.
t, err := dec.Token()
if err != nil {
return xerrors.Errorf("read array open bracket: %w", err)
}
if t != json.Delim('[') {
return xerrors.Errorf("expected array open bracket, got %q", t)
}
// As long as json elements exist, consume them. The counter is used for
// better errors.
var i int
OptionSetDecodeLoop:
for dec.More() {
var opt Option
// jValue is a placeholder value that allows us to capture the
// raw json for the value to attempt to unmarshal later.
var jValue jsonValue
opt.Value = &jValue
err := dec.Decode(&opt)
if err != nil {
return xerrors.Errorf("decode %d option: %w", i, err)
}
// This counter is used to contextualize errors to show which element of
// the array we failed to decode. It is only used in the error above, as
// if the above works, we can instead use the Option.Name which is more
// descriptive and useful. So increment here for the next decode.
i++
// Try to see if the option already exists in the option set.
// If it does, just update the existing option.
for optIndex, have := range *optSet {
if have.Name == opt.Name {
if jValue != nil {
err := json.Unmarshal(jValue, &(*optSet)[optIndex].Value)
if err != nil {
return xerrors.Errorf("decode option %q value: %w", have.Name, err)
}
// Set the opt's value
opt.Value = (*optSet)[optIndex].Value
} else {
// Hopefully the user passed empty values in the option set. There is no easy way
// to tell, and if we do not do this, it breaks json.Marshal if we do it again on
// this new option set.
opt.Value = (*optSet)[optIndex].Value
}
// Override the existing.
(*optSet)[optIndex] = opt
// Go to the next option to decode.
continue OptionSetDecodeLoop
}
}
// If the option doesn't exist, the value will be discarded.
// We do this because we cannot infer the type of the value.
opt.Value = DiscardValue
*optSet = append(*optSet, opt)
}
t, err = dec.Token()
if err != nil {
return xerrors.Errorf("read array close bracket: %w", err)
}
if t != json.Delim(']') {
return xerrors.Errorf("expected array close bracket, got %q", t)
}
return nil
}
// Add adds the given Options to the OptionSet.
func (s *OptionSet) Add(opts ...Option) {
*s = append(*s, opts...)
func (optSet *OptionSet) Add(opts ...Option) {
*optSet = append(*optSet, opts...)
}
// Filter will only return options that match the given filter. (return true)
func (s OptionSet) Filter(filter func(opt Option) bool) OptionSet {
func (optSet OptionSet) Filter(filter func(opt Option) bool) OptionSet {
cpy := make(OptionSet, 0)
for _, opt := range s {
for _, opt := range optSet {
if filter(opt) {
cpy = append(cpy, opt)
}
@ -96,13 +198,13 @@ func (s OptionSet) Filter(filter func(opt Option) bool) OptionSet {
}
// FlagSet returns a pflag.FlagSet for the OptionSet.
func (s *OptionSet) FlagSet() *pflag.FlagSet {
if s == nil {
func (optSet *OptionSet) FlagSet() *pflag.FlagSet {
if optSet == nil {
return &pflag.FlagSet{}
}
fs := pflag.NewFlagSet("", pflag.ContinueOnError)
for _, opt := range *s {
for _, opt := range *optSet {
if opt.Flag == "" {
continue
}
@ -139,8 +241,8 @@ func (s *OptionSet) FlagSet() *pflag.FlagSet {
// ParseEnv parses the given environment variables into the OptionSet.
// Use EnvsWithPrefix to filter out prefixes.
func (s *OptionSet) ParseEnv(vs []EnvVar) error {
if s == nil {
func (optSet *OptionSet) ParseEnv(vs []EnvVar) error {
if optSet == nil {
return nil
}
@ -154,7 +256,7 @@ func (s *OptionSet) ParseEnv(vs []EnvVar) error {
envs[v.Name] = v.Value
}
for i, opt := range *s {
for i, opt := range *optSet {
if opt.Env == "" {
continue
}
@ -172,7 +274,7 @@ func (s *OptionSet) ParseEnv(vs []EnvVar) error {
continue
}
(*s)[i].ValueSource = ValueSourceEnv
(*optSet)[i].ValueSource = ValueSourceEnv
if err := opt.Value.Set(envVal); err != nil {
merr = multierror.Append(
merr, xerrors.Errorf("parse %q: %w", opt.Name, err),
@ -185,14 +287,14 @@ func (s *OptionSet) ParseEnv(vs []EnvVar) error {
// SetDefaults sets the default values for each Option, skipping values
// that already have a value source.
func (s *OptionSet) SetDefaults() error {
if s == nil {
func (optSet *OptionSet) SetDefaults() error {
if optSet == nil {
return nil
}
var merr *multierror.Error
for i, opt := range *s {
for i, opt := range *optSet {
// Skip values that may have already been set by the user.
if opt.ValueSource != ValueSourceNone {
continue
@ -212,7 +314,7 @@ func (s *OptionSet) SetDefaults() error {
)
continue
}
(*s)[i].ValueSource = ValueSourceDefault
(*optSet)[i].ValueSource = ValueSourceDefault
if err := opt.Value.Set(opt.Default); err != nil {
merr = multierror.Append(
merr, xerrors.Errorf("parse %q: %w", opt.Name, err),
@ -224,9 +326,9 @@ func (s *OptionSet) SetDefaults() error {
// ByName returns the Option with the given name, or nil if no such option
// exists.
func (s *OptionSet) ByName(name string) *Option {
for i := range *s {
opt := &(*s)[i]
func (optSet *OptionSet) ByName(name string) *Option {
for i := range *optSet {
opt := &(*optSet)[i]
if opt.Name == name {
return opt
}

168
cli/clibase/option_test.go
View File

@ -1,11 +1,17 @@
package clibase_test
import (
"bytes"
"encoding/json"
"regexp"
"testing"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"github.com/coder/coder/v2/cli/clibase"
"github.com/coder/coder/v2/coderd/coderdtest"
"github.com/coder/coder/v2/codersdk"
)
func TestOptionSet_ParseFlags(t *testing.T) {
@ -201,3 +207,165 @@ func TestOptionSet_ParseEnv(t *testing.T) {
require.EqualValues(t, expected, actual.Value)
})
}
func TestOptionSet_JsonMarshal(t *testing.T) {
t.Parallel()
// This unit test ensures if the source optionset is missing the option
// and cannot determine the type, it will not panic. The unmarshal will
// succeed with a best effort.
t.Run("MissingSrcOption", func(t *testing.T) {
t.Parallel()
var str clibase.String = "something"
var arr clibase.StringArray = []string{"foo", "bar"}
opts := clibase.OptionSet{
clibase.Option{
Name: "StringOpt",
Value: &str,
},
clibase.Option{
Name: "ArrayOpt",
Value: &arr,
},
}
data, err := json.Marshal(opts)
require.NoError(t, err, "marshal option set")
tgt := clibase.OptionSet{}
err = json.Unmarshal(data, &tgt)
require.NoError(t, err, "unmarshal option set")
for i := range opts {
compareOptionsExceptValues(t, opts[i], tgt[i])
require.Empty(t, tgt[i].Value.String(), "unknown value types are empty")
}
})
t.Run("RegexCase", func(t *testing.T) {
t.Parallel()
val := clibase.Regexp(*regexp.MustCompile(".*"))
opts := clibase.OptionSet{
clibase.Option{
Name: "Regex",
Value: &val,
Default: ".*",
},
}
data, err := json.Marshal(opts)
require.NoError(t, err, "marshal option set")
var foundVal clibase.Regexp
newOpts := clibase.OptionSet{
clibase.Option{
Name: "Regex",
Value: &foundVal,
},
}
err = json.Unmarshal(data, &newOpts)
require.NoError(t, err, "unmarshal option set")
require.EqualValues(t, opts[0].Value.String(), newOpts[0].Value.String())
})
t.Run("AllValues", func(t *testing.T) {
t.Parallel()
vals := coderdtest.DeploymentValues(t)
opts := vals.Options()
sources := []clibase.ValueSource{
clibase.ValueSourceNone,
clibase.ValueSourceFlag,
clibase.ValueSourceEnv,
clibase.ValueSourceYAML,
clibase.ValueSourceDefault,
}
for i := range opts {
opts[i].ValueSource = sources[i%len(sources)]
}
data, err := json.Marshal(opts)
require.NoError(t, err, "marshal option set")
newOpts := (&codersdk.DeploymentValues{}).Options()
err = json.Unmarshal(data, &newOpts)
require.NoError(t, err, "unmarshal option set")
for i := range opts {
exp := opts[i]
found := newOpts[i]
compareOptionsExceptValues(t, exp, found)
compareValues(t, exp, found)
}
thirdOpts := (&codersdk.DeploymentValues{}).Options()
data, err = json.Marshal(newOpts)
require.NoError(t, err, "marshal option set")
err = json.Unmarshal(data, &thirdOpts)
require.NoError(t, err, "unmarshal option set")
// Compare to the original opts again
for i := range opts {
exp := opts[i]
found := thirdOpts[i]
compareOptionsExceptValues(t, exp, found)
compareValues(t, exp, found)
}
})
}
func compareOptionsExceptValues(t *testing.T, exp, found clibase.Option) {
t.Helper()
require.Equalf(t, exp.Name, found.Name, "option name %q", exp.Name)
require.Equalf(t, exp.Description, found.Description, "option description %q", exp.Name)
require.Equalf(t, exp.Required, found.Required, "option required %q", exp.Name)
require.Equalf(t, exp.Flag, found.Flag, "option flag %q", exp.Name)
require.Equalf(t, exp.FlagShorthand, found.FlagShorthand, "option flag shorthand %q", exp.Name)
require.Equalf(t, exp.Env, found.Env, "option env %q", exp.Name)
require.Equalf(t, exp.YAML, found.YAML, "option yaml %q", exp.Name)
require.Equalf(t, exp.Default, found.Default, "option default %q", exp.Name)
require.Equalf(t, exp.ValueSource, found.ValueSource, "option value source %q", exp.Name)
require.Equalf(t, exp.Hidden, found.Hidden, "option hidden %q", exp.Name)
require.Equalf(t, exp.Annotations, found.Annotations, "option annotations %q", exp.Name)
require.Equalf(t, exp.Group, found.Group, "option group %q", exp.Name)
// UseInstead is the same comparison problem, just check the length
require.Equalf(t, len(exp.UseInstead), len(found.UseInstead), "option use instead %q", exp.Name)
}
func compareValues(t *testing.T, exp, found clibase.Option) {
t.Helper()
if (exp.Value == nil || found.Value == nil) || (exp.Value.String() != found.Value.String() && found.Value.String() == "") {
// If the string values are different, this can be a "nil" issue.
// So only run this case if the found string is the empty string.
// We use MarshalYAML for struct strings, and it will return an
// empty string '""' for nil slices/maps/etc.
// So use json to compare.
expJSON, err := json.Marshal(exp.Value)
require.NoError(t, err, "marshal")
foundJSON, err := json.Marshal(found.Value)
require.NoError(t, err, "marshal")
expJSON = normalizeJSON(expJSON)
foundJSON = normalizeJSON(foundJSON)
assert.Equalf(t, string(expJSON), string(foundJSON), "option value %q", exp.Name)
} else {
assert.Equal(t,
exp.Value.String(),
found.Value.String(),
"option value %q", exp.Name)
}
}
// normalizeJSON handles the fact that an empty map/slice is not the same
// as a nil empty/slice. For our purposes, they are the same.
func normalizeJSON(data []byte) []byte {
if bytes.Equal(data, []byte("[]")) || bytes.Equal(data, []byte("{}")) {
return []byte("null")
}
return data
}

48
cli/clibase/values.go
View File

@ -430,6 +430,35 @@ func (discardValue) Type() string {
return "discard"
}
func (discardValue) UnmarshalJSON([]byte) error {
return nil
}
// jsonValue is intentionally not exported. It is just used to store the raw JSON
// data for a value to defer it's unmarshal. It implements the pflag.Value to be
// usable in an Option.
type jsonValue json.RawMessage
func (jsonValue) Set(string) error {
return xerrors.Errorf("json value is read-only")
}
func (jsonValue) String() string {
return ""
}
func (jsonValue) Type() string {
return "json"
}
func (j *jsonValue) UnmarshalJSON(data []byte) error {
if j == nil {
return xerrors.New("json.RawMessage: UnmarshalJSON on nil pointer")
}
*j = append((*j)[0:0], data...)
return nil
}
var _ pflag.Value = (*Enum)(nil)
type Enum struct {
@ -464,6 +493,25 @@ func (e *Enum) String() string {
type Regexp regexp.Regexp
func (r *Regexp) MarshalJSON() ([]byte, error) {
return json.Marshal(r.String())
}
func (r *Regexp) UnmarshalJSON(data []byte) error {
var source string
err := json.Unmarshal(data, &source)
if err != nil {
return err
}
exp, err := regexp.Compile(source)
if err != nil {
return xerrors.Errorf("invalid regex expression: %w", err)
}
*r = Regexp(*exp)
return nil
}
func (r *Regexp) MarshalYAML() (interface{}, error) {
return yaml.Node{
Kind: yaml.ScalarNode,

10
cli/clibase/yaml.go
View File

@ -51,12 +51,12 @@ func deepMapNode(n *yaml.Node, path []string, headComment string) *yaml.Node {
// the stack.
//
// It is isomorphic with FromYAML.
func (s *OptionSet) MarshalYAML() (any, error) {
func (optSet *OptionSet) MarshalYAML() (any, error) {
root := yaml.Node{
Kind: yaml.MappingNode,
}
for _, opt := range *s {
for _, opt := range *optSet {
if opt.YAML == "" {
continue
}
@ -222,7 +222,7 @@ func (o *Option) setFromYAMLNode(n *yaml.Node) error {
// UnmarshalYAML converts the given YAML node into the option set.
// It is isomorphic with ToYAML.
func (s *OptionSet) UnmarshalYAML(rootNode *yaml.Node) error {
func (optSet *OptionSet) UnmarshalYAML(rootNode *yaml.Node) error {
// The rootNode will be a DocumentNode if it's read from a file. We do
// not support multiple documents in a single file.
if rootNode.Kind == yaml.DocumentNode {
@ -240,8 +240,8 @@ func (s *OptionSet) UnmarshalYAML(rootNode *yaml.Node) error {
matchedNodes := make(map[string]*yaml.Node, len(yamlNodes))
var merr error
for i := range *s {
opt := &(*s)[i]
for i := range *optSet {
opt := &(*optSet)[i]
if opt.YAML == "" {
continue
}

4
cli/server.go
View File

@ -617,6 +617,10 @@ func (r *RootCmd) Server(newAPI func(context.Context, *coderd.Options) (*coderd.
MetricsCacheRefreshInterval: vals.MetricsCacheRefreshInterval.Value(),
AgentStatsRefreshInterval: vals.AgentStatRefreshInterval.Value(),
DeploymentValues: vals,
// Do not pass secret values to DeploymentOptions. All values should be read from
// the DeploymentValues instead, this just serves to indicate the source of each
// option. This is just defensive to prevent accidentally leaking.
DeploymentOptions: codersdk.DeploymentOptionsWithoutSecrets(opts),
PrometheusRegistry: prometheus.NewRegistry(),
APIRateLimit: int(vals.RateLimit.API.Value()),
LoginRateLimit: loginRateLimit,

19
cli/server_test.go
View File

@ -1188,6 +1188,22 @@ func TestServer(t *testing.T) {
require.Equal(t, map[string]string{"serious_business_unit": "serious_business_unit"}, deploymentConfig.Values.OIDC.GroupMapping.Value)
require.Equal(t, "Sign In With Coder", deploymentConfig.Values.OIDC.SignInText.Value())
require.Equal(t, "https://example.com/icon.png", deploymentConfig.Values.OIDC.IconURL.Value().String())
// Verify the option values
for _, opt := range deploymentConfig.Options {
switch opt.Flag {
case "access-url":
require.Equal(t, "http://example.com", opt.Value.String())
case "oidc-icon-url":
require.Equal(t, "https://example.com/icon.png", opt.Value.String())
case "oidc-sign-in-text":
require.Equal(t, "Sign In With Coder", opt.Value.String())
case "redirect-to-access-url":
require.Equal(t, "false", opt.Value.String())
case "derp-server-region-id":
require.Equal(t, "999", opt.Value.String())
}
}
})
})
@ -1488,6 +1504,9 @@ func TestServer(t *testing.T) {
gotConfig.Options.ByName("Config Path").Value.Set("")
// We check the options individually for better error messages.
for i := range wantConfig.Options {
// ValueSource is not going to be correct on the `want`, so just
// match that field.
wantConfig.Options[i].ValueSource = gotConfig.Options[i].ValueSource
assert.Equal(
t, wantConfig.Options[i],
gotConfig.Options[i],

6
coderd/coderd.go
View File

@ -40,6 +40,7 @@ import (
"cdr.dev/slog"
"github.com/coder/coder/v2/buildinfo"
"github.com/coder/coder/v2/cli/clibase"
"github.com/coder/coder/v2/coderd/audit"
"github.com/coder/coder/v2/coderd/awsidentity"
"github.com/coder/coder/v2/coderd/batchstats"
@ -152,6 +153,11 @@ type Options struct {
MetricsCacheRefreshInterval time.Duration
AgentStatsRefreshInterval time.Duration
DeploymentValues *codersdk.DeploymentValues
// DeploymentOptions do contain the copy of DeploymentValues, and contain
// contextual information about how the values were set.
// Do not use DeploymentOptions to retrieve values, use DeploymentValues instead.
// All secrets values are stripped.
DeploymentOptions clibase.OptionSet
UpdateCheckOptions *updatecheck.Options // Set non-nil to enable update checking.
// SSHConfig is the response clients use to configure config-ssh locally.

1
coderd/coderdtest/coderdtest.go
View File

@ -417,6 +417,7 @@ func NewOptions(t testing.TB, options *Options) (func(http.Handler), context.Can
MetricsCacheRefreshInterval: options.MetricsCacheRefreshInterval,
AgentStatsRefreshInterval: options.AgentStatsRefreshInterval,
DeploymentValues: options.DeploymentValues,
DeploymentOptions: codersdk.DeploymentOptionsWithoutSecrets(options.DeploymentValues.Options()),
UpdateCheckOptions: options.UpdateCheckOptions,
SwaggerEndpoint: options.SwaggerEndpoint,
AppSecurityKey: AppSecurityKey,

2
coderd/deployment.go
View File

@ -33,7 +33,7 @@ func (api *API) deploymentValues(rw http.ResponseWriter, r *http.Request) {
r.Context(), rw, http.StatusOK,
codersdk.DeploymentConfig{
Values: values,
Options: values.Options(),
Options: api.DeploymentOptions,
},
)
}

13
codersdk/deployment.go
View File

@ -1762,6 +1762,19 @@ type LinkConfig struct {
Icon string `json:"icon" yaml:"icon"`
}
// DeploymentOptionsWithoutSecrets returns a copy of the OptionSet with secret values omitted.
func DeploymentOptionsWithoutSecrets(set clibase.OptionSet) clibase.OptionSet {
cpy := make(clibase.OptionSet, 0, len(set))
for _, opt := range set {
cpyOpt := opt
if IsSecretDeploymentOption(cpyOpt) {
cpyOpt.Value = nil
}
cpy = append(cpy, cpyOpt)
}
return cpy
}
// WithoutSecrets returns a copy of the config without secret values.
func (c *DeploymentValues) WithoutSecrets() (*DeploymentValues, error) {
var ff DeploymentValues

25
site/src/components/DeploySettingsLayout/Option.tsx
View File

@ -3,6 +3,7 @@ import { PropsWithChildren, FC } from "react";
import { MONOSPACE_FONT_FAMILY } from "theme/constants";
import { DisabledBadge, EnabledBadge } from "./Badges";
import Box, { BoxProps } from "@mui/material/Box";
import { combineClasses } from "utils/combineClasses";
export const OptionName: FC<PropsWithChildren> = ({ children }) => {
const styles = useStyles();
@ -58,21 +59,34 @@ export const OptionValue: FC<{ children?: unknown }> = ({ children }) => {
return <span className={styles.optionValue}>{JSON.stringify(children)}</span>;
};
export const OptionConfig = (props: BoxProps) => {
// OptionalConfig takes a source bool to indicate if the Option is the source of the configured value.
export const OptionConfig = ({
source,
...boxProps
}: { source?: boolean } & BoxProps) => {
return (
<Box
{...props}
{...boxProps}
sx={{
fontSize: 13,
fontFamily: MONOSPACE_FONT_FAMILY,
fontWeight: 600,
backgroundColor: (theme) => theme.palette.background.paperLight,
backgroundColor: (theme) =>
source
? theme.palette.primary.dark
: theme.palette.background.paperLight,
display: "inline-flex",
alignItems: "center",
borderRadius: 0.25,
padding: (theme) => theme.spacing(0, 1),
border: (theme) => `1px solid ${theme.palette.divider}`,
...props.sx,
border: (theme) =>
`1px solid ${
source ? theme.palette.primary.main : theme.palette.divider
}`,
"& .option-config-flag": {
backgroundColor: source ? "rgba(0, 0, 0, 0.7)" : undefined,
},
...boxProps.sx,
}}
/>
);
@ -82,6 +96,7 @@ export const OptionConfigFlag = (props: BoxProps) => {
return (
<Box
{...props}
className={combineClasses([props.className, "option-config-flag"])}
sx={{
fontSize: 10,
fontWeight: 600,

8
site/src/components/DeploySettingsLayout/OptionsTable.tsx
View File

@ -58,25 +58,25 @@ const OptionsTable: FC<{
}}
>
{option.flag && (
<OptionConfig>
<OptionConfig source={option.value_source === "flag"}>
<OptionConfigFlag>CLI</OptionConfigFlag>
--{option.flag}
</OptionConfig>
)}
{option.flag_shorthand && (
<OptionConfig>
<OptionConfig source={option.value_source === "flag"}>
<OptionConfigFlag>CLI</OptionConfigFlag>-
{option.flag_shorthand}
</OptionConfig>
)}
{option.env && (
<OptionConfig>
<OptionConfig source={option.value_source === "env"}>
<OptionConfigFlag>ENV</OptionConfigFlag>
{option.env}
</OptionConfig>
)}
{option.yaml && (
<OptionConfig>
<OptionConfig source={option.value_source === "yaml"}>
<OptionConfigFlag>YAML</OptionConfigFlag>
{option.yaml}
</OptionConfig>