From 13d5129b56c369525f9561ba3024f7cc497af233 Mon Sep 17 00:00:00 2001 From: Lars Karlitski Date: Fri, 25 Sep 2020 11:20:09 +0200 Subject: [PATCH] osbuild-composer: use less pointers in config The default values of fields in both ComposerConfig.Koji and ComposerConfig.Worker are well-suited for how they're used. The nil-checks in main.go only checked that the sections exist. This is quite a weak check for validity, because the sections could be empty. If anything is required for composer to function, we could add proper validation in the future. Do the same for the CA fields, which contain file names. Go has lots of precedent for using empty strings to denote "no value" in the standard library. Use it for CA files, too, instead of pointers. --- cmd/osbuild-composer/config.go | 10 +++++----- cmd/osbuild-composer/config_test.go | 26 +++++++++++++++++++++++-- cmd/osbuild-composer/main.go | 16 +++++---------- cmd/osbuild-composer/testdata/test.toml | 11 +++++++++++ 4 files changed, 45 insertions(+), 18 deletions(-) create mode 100644 cmd/osbuild-composer/testdata/test.toml diff --git a/cmd/osbuild-composer/config.go b/cmd/osbuild-composer/config.go index 2c600a659..38bec22a9 100644 --- a/cmd/osbuild-composer/config.go +++ b/cmd/osbuild-composer/config.go @@ -7,7 +7,7 @@ import ( ) type ComposerConfigFile struct { - Koji *struct { + Koji struct { Servers map[string]struct { Kerberos *struct { Principal string `toml:"principal"` @@ -15,12 +15,12 @@ type ComposerConfigFile struct { } `toml:"kerberos,omitempty"` } `toml:"servers"` AllowedDomains []string `toml:"allowed_domains"` - CA *string `toml:"ca"` + CA string `toml:"ca"` } `toml:"koji"` - Worker *struct { + Worker struct { AllowedDomains []string `toml:"allowed_domains"` - CA *string `toml:"ca"` - } `toml:"worker,omitempty"` + CA string `toml:"ca"` + } `toml:"worker"` } func LoadConfig(name string) (*ComposerConfigFile, error) { diff --git a/cmd/osbuild-composer/config_test.go b/cmd/osbuild-composer/config_test.go index 8b1459fda..f5341e1bf 100644 --- a/cmd/osbuild-composer/config_test.go +++ b/cmd/osbuild-composer/config_test.go @@ -10,8 +10,12 @@ import ( func TestEmpty(t *testing.T) { config, err := LoadConfig("testdata/empty-config.toml") require.NoError(t, err) - require.Nil(t, config.Koji) - require.Nil(t, config.Worker) + require.NotNil(t, config) + require.Empty(t, config.Koji.Servers) + require.Empty(t, config.Koji.AllowedDomains) + require.Empty(t, config.Koji.CA) + require.Empty(t, config.Worker.AllowedDomains) + require.Empty(t, config.Worker.CA) } func TestNonExisting(t *testing.T) { @@ -20,3 +24,21 @@ func TestNonExisting(t *testing.T) { require.True(t, os.IsNotExist(err)) require.Nil(t, config) } + +func TestConfig(t *testing.T) { + config, err := LoadConfig("testdata/test.toml") + require.NoError(t, err) + require.NotNil(t, config) + + server, ok := config.Koji.Servers["example.com"] + require.True(t, ok) + require.NotNil(t, server.Kerberos) + require.Equal(t, server.Kerberos.Principal, "example@osbuild.org") + require.Equal(t, server.Kerberos.KeyTab, "/etc/osbuild-composer/osbuild.keytab") + + require.Equal(t, config.Koji.AllowedDomains, []string{"osbuild.org"}) + require.Equal(t, config.Koji.CA, "/etc/osbuild-composer/ca-crt.pem") + + require.Equal(t, config.Worker.AllowedDomains, []string{"osbuild.org"}) + require.Equal(t, config.Worker.CA, "/etc/osbuild-composer/ca-crt.pem") +} diff --git a/cmd/osbuild-composer/main.go b/cmd/osbuild-composer/main.go index c8ae2bf78..2e903fc1c 100644 --- a/cmd/osbuild-composer/main.go +++ b/cmd/osbuild-composer/main.go @@ -30,9 +30,10 @@ import ( const configFile = "/etc/osbuild-composer/osbuild-composer.toml" type connectionConfig struct { - // CA used for client certificate validation. If nil, then the CAs + // CA used for client certificate validation. If empty, then the CAs // trusted by the host system are used. - CACertFile *string + CACertFile string + ServerKeyFile string ServerCertFile string AllowedDomains []string @@ -41,8 +42,8 @@ type connectionConfig struct { func createTLSConfig(c *connectionConfig) (*tls.Config, error) { var roots *x509.CertPool - if c.CACertFile != nil { - caCertPEM, err := ioutil.ReadFile(*c.CACertFile) + if c.CACertFile != "" { + caCertPEM, err := ioutil.ReadFile(c.CACertFile) if err != nil { return nil, err } @@ -200,9 +201,6 @@ func main() { // Optionally run Koji API if kojiListeners, exists := listeners["osbuild-composer-koji.socket"]; exists { - if config.Koji == nil { - log.Fatal("koji not configured in the config file") - } kojiServers := make(map[string]koji.GSSAPICredentials) for server, creds := range config.Koji.Servers { if creds.Kerberos == nil { @@ -248,10 +246,6 @@ func main() { log.Printf("Starting remote listener\n") - if config.Worker == nil { - log.Fatal("remote worker not configured in the config file") - } - tlsConfig, err := createTLSConfig(&connectionConfig{ CACertFile: config.Worker.CA, ServerKeyFile: "/etc/osbuild-composer/composer-key.pem", diff --git a/cmd/osbuild-composer/testdata/test.toml b/cmd/osbuild-composer/testdata/test.toml new file mode 100644 index 000000000..2a81ab237 --- /dev/null +++ b/cmd/osbuild-composer/testdata/test.toml @@ -0,0 +1,11 @@ +[koji] +allowed_domains = [ "osbuild.org" ] +ca = "/etc/osbuild-composer/ca-crt.pem" + +[koji.servers."example.com".kerberos] +principal = "example@osbuild.org" +keytab = "/etc/osbuild-composer/osbuild.keytab" + +[worker] +allowed_domains = [ "osbuild.org" ] +ca = "/etc/osbuild-composer/ca-crt.pem"