Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add default configuration values #316

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
+19 −8
Open

Add default configuration values #316

Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
f933203
Add default configuration values
AdamBark Jul 24, 2020
6bdc6f1
Apply configuration defaults from a file
AdamBark Jul 27, 2020
6f83814
Update cmd/pebble/main.go
AdamBark Jul 27, 2020
193f076
Add check for default config existence
AdamBark Jul 29, 2020
9590ec4
Update default config naming
AdamBark Jul 31, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 12 additions & 1 deletion cmd/pebble/main.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,17 @@ type config struct {
}
}

func setConfigDefaults() (config) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: because this is creating a new config object, rather than setting values on a pre-existing config object, I'd call it getDefaultConfig rather than setConfigDefaults.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's what you get for not updating your function names when you change what it does 🤦

var conf config
conf.Pebble.ListenAddress = "0.0.0.0:14000"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add a comment showing readers that these values are identical to the values in /test/config/pebble-config.json, and that they should be kept in sync.

conf.Pebble.ManagementListenAddress = "0.0.0.0:15000"
conf.Pebble.Certificate = "test/certs/localhost/cert.pem"
conf.Pebble.PrivateKey = "test/certs/localhost/key.pem"
conf.Pebble.HTTPPort = 5002
conf.Pebble.TLSPort = 5001
return conf
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are two values (ocspResponderUrl and externalAccountBindingRequired) which don't get initialized here. Sure, their defaults are also their zero-values, but being explicit is good.

}

func main() {
configFile := flag.String(
"config",
Expand All @@ -52,7 +63,7 @@ func main() {
logger := log.New(os.Stdout, "Pebble ", log.LstdFlags)
logger.Printf("Starting Pebble ACME server")

var c config
c := setConfigDefaults()
err := cmd.ReadConfigFile(*configFile, &c)
cmd.FailOnError(err, "Reading JSON config file into config structure")

Expand Down