Skip to content

Commit

Permalink
refactor(log): replace panics with log fatal or log panic functions
Browse files Browse the repository at this point in the history
Signed-off-by: Laurentiu Niculae <[email protected]>
  • Loading branch information
laurentiuNiculae committed Aug 28, 2023
1 parent 28858f6 commit 064d471
Show file tree
Hide file tree
Showing 22 changed files with 196 additions and 129 deletions.
1 change: 1 addition & 0 deletions errors/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -151,4 +151,5 @@ var (
ErrInvalidPublicKeyContent = errors.New("signatures: invalid public key content")
ErrInvalidStateCookie = errors.New("auth: state cookie not present or differs from original state")
ErrSyncNoURLsLeft = errors.New("sync: no valid registry urls left after filtering local ones")
ErrServerIsRunning = errors.New("server is running")
)
41 changes: 24 additions & 17 deletions pkg/api/authn.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,10 +51,11 @@ const (
type AuthnMiddleware struct {
credMap map[string]string
ldapClient *LDAPClient
log log.Logger
}

func AuthHandler(ctlr *Controller) mux.MiddlewareFunc {
authnMiddleware := &AuthnMiddleware{}
authnMiddleware := &AuthnMiddleware{log: ctlr.Log}

if ctlr.Config.IsBearerAuthEnabled() {
return bearerAuthHandler(ctlr)
Expand Down Expand Up @@ -245,14 +246,14 @@ func (amw *AuthnMiddleware) TryAuthnHandlers(ctlr *Controller) mux.MiddlewareFun

cookieStoreHashKey := securecookie.GenerateRandomKey(64)
if cookieStoreHashKey == nil {
panic(zerr.ErrHashKeyNotCreated)
amw.log.Panic().Err(zerr.ErrHashKeyNotCreated).Msg("failed to generate random key")
}

// if storage is filesystem then use zot's rootDir to store sessions
if ctlr.Config.Storage.StorageDriver == nil {
sessionsDir := path.Join(ctlr.Config.Storage.RootDirectory, "_sessions")
if err := os.MkdirAll(sessionsDir, storageConstants.DefaultDirPerms); err != nil {
panic(err)
amw.log.Panic().Err(err).Str("session-dir", sessionsDir).Msg("failed to create session dir")
}

cookieStore := sessions.NewFilesystemStore(sessionsDir, cookieStoreHashKey)
Expand Down Expand Up @@ -291,21 +292,24 @@ func (amw *AuthnMiddleware) TryAuthnHandlers(ctlr *Controller) mux.MiddlewareFun
if ctlr.Config.HTTP.Auth.LDAP.CACert != "" {
caCert, err := os.ReadFile(ctlr.Config.HTTP.Auth.LDAP.CACert)
if err != nil {
panic(err)
amw.log.Panic().Err(err).Str("caCert", ctlr.Config.HTTP.Auth.LDAP.CACert).
Msg("failed to read caCert")
}

caCertPool := x509.NewCertPool()

if !caCertPool.AppendCertsFromPEM(caCert) {
panic(zerr.ErrBadCACert)
amw.log.Panic().Err(zerr.ErrBadCACert).Str("caCert", ctlr.Config.HTTP.Auth.LDAP.CACert).
Msg("failed to read caCert")
}

amw.ldapClient.ClientCAs = caCertPool
} else {
// default to system cert pool
caCertPool, err := x509.SystemCertPool()
if err != nil {
panic(zerr.ErrBadCACert)
amw.log.Panic().Err(zerr.ErrBadCACert).Str("caCert", ctlr.Config.HTTP.Auth.LDAP.CACert).
Msg("failed to get system cert pool")
}

amw.ldapClient.ClientCAs = caCertPool
Expand All @@ -315,7 +319,8 @@ func (amw *AuthnMiddleware) TryAuthnHandlers(ctlr *Controller) mux.MiddlewareFun
if ctlr.Config.IsHtpasswdAuthEnabled() {
credsFile, err := os.Open(ctlr.Config.HTTP.Auth.HTPasswd.Path)
if err != nil {
panic(err)
amw.log.Panic().Err(err).Str("credsFile", ctlr.Config.HTTP.Auth.HTPasswd.Path).
Msg("failed to open creds-file")
}
defer credsFile.Close()

Expand All @@ -336,10 +341,10 @@ func (amw *AuthnMiddleware) TryAuthnHandlers(ctlr *Controller) mux.MiddlewareFun

for provider := range ctlr.Config.HTTP.Auth.OpenID.Providers {
if config.IsOpenIDSupported(provider) {
rp := NewRelyingPartyOIDC(ctlr.Config, provider)
rp := NewRelyingPartyOIDC(ctlr.Config, provider, ctlr.Log)
ctlr.RelyingParties[provider] = rp
} else if config.IsOauth2Supported(provider) {
rp := NewRelyingPartyGithub(ctlr.Config, provider)
rp := NewRelyingPartyGithub(ctlr.Config, provider, ctlr.Log)
ctlr.RelyingParties[provider] = rp
}
}
Expand Down Expand Up @@ -548,19 +553,20 @@ func (rh *RouteHandler) AuthURLHandler() http.HandlerFunc {
}
}

func NewRelyingPartyOIDC(config *config.Config, provider string) rp.RelyingParty {
issuer, clientID, clientSecret, redirectURI, scopes, options := getRelyingPartyArgs(config, provider)
func NewRelyingPartyOIDC(config *config.Config, provider string, log log.Logger) rp.RelyingParty {
issuer, clientID, clientSecret, redirectURI, scopes, options := getRelyingPartyArgs(config, provider, log)

relyingParty, err := rp.NewRelyingPartyOIDC(issuer, clientID, clientSecret, redirectURI, scopes, options...)
if err != nil {
panic(err)
log.Panic().Err(err).Str("issuer", issuer).Str("redirectURI", redirectURI).Strs("scopes", scopes).
Msg("failed to get new relying party oicd")
}

return relyingParty
}

func NewRelyingPartyGithub(config *config.Config, provider string) rp.RelyingParty {
_, clientID, clientSecret, redirectURI, scopes, options := getRelyingPartyArgs(config, provider)
func NewRelyingPartyGithub(config *config.Config, provider string, log log.Logger) rp.RelyingParty {
_, clientID, clientSecret, redirectURI, scopes, options := getRelyingPartyArgs(config, provider, log)

rpConfig := &oauth2.Config{
ClientID: clientID,
Expand All @@ -572,17 +578,18 @@ func NewRelyingPartyGithub(config *config.Config, provider string) rp.RelyingPar

relyingParty, err := rp.NewRelyingPartyOAuth(rpConfig, options...)
if err != nil {
panic(err)
log.Panic().Err(err).Str("redirectURI", redirectURI).Strs("scopes", scopes).
Msg("failed to get new relying party oauth")
}

return relyingParty
}

func getRelyingPartyArgs(cfg *config.Config, provider string) (
func getRelyingPartyArgs(cfg *config.Config, provider string, log log.Logger) (
string, string, string, string, []string, []rp.Option,
) {
if _, ok := cfg.HTTP.Auth.OpenID.Providers[provider]; !ok {
panic(zerr.ErrOpenIDProviderDoesNotExist)
log.Panic().Err(zerr.ErrOpenIDProviderDoesNotExist).Str("provider", provider).Msg("")
}

clientID := cfg.HTTP.Auth.OpenID.Providers[provider].ClientID
Expand Down
8 changes: 6 additions & 2 deletions pkg/api/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -188,13 +188,17 @@ func (c *Controller) Run(reloadCtx context.Context) error {

caCert, err := os.ReadFile(c.Config.HTTP.TLS.CACert)
if err != nil {
panic(err)
c.Log.Error().Err(err).Str("caCert", c.Config.HTTP.TLS.CACert).Msg("failed to read file")

return err
}

caCertPool := x509.NewCertPool()

if !caCertPool.AppendCertsFromPEM(caCert) {
panic(errors.ErrBadCACert)
c.Log.Error().Err(errors.ErrBadCACert).Msg("failed to append certs from pem")

return errors.ErrBadCACert
}

server.TLSConfig.ClientAuth = clientAuth
Expand Down
27 changes: 16 additions & 11 deletions pkg/api/controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -131,13 +131,15 @@ func TestCreateCacheDatabaseDriver(t *testing.T) {
panic(err)
}

driver := storage.CreateCacheDatabaseDriver(conf.Storage.StorageConfig, log)
driver, err := storage.CreateCacheDatabaseDriver(conf.Storage.StorageConfig, log)
So(err, ShouldNotBeNil)
So(driver, ShouldBeNil)

conf.Storage.RemoteCache = true
conf.Storage.RootDirectory = t.TempDir()

driver = storage.CreateCacheDatabaseDriver(conf.Storage.StorageConfig, log)
driver, err = storage.CreateCacheDatabaseDriver(conf.Storage.StorageConfig, log)
So(err, ShouldBeNil)
So(driver, ShouldBeNil)
})
skipDynamo(t)
Expand Down Expand Up @@ -170,8 +172,9 @@ func TestCreateCacheDatabaseDriver(t *testing.T) {
"versionTablename": "Version",
}

driver := storage.CreateCacheDatabaseDriver(conf.Storage.StorageConfig, log)
So(driver, ShouldNotBeNil)
driver, err := storage.CreateCacheDatabaseDriver(conf.Storage.StorageConfig, log)
So(err, ShouldNotBeNil)
So(driver, ShouldBeNil)

// negative test cases

Expand All @@ -185,7 +188,8 @@ func TestCreateCacheDatabaseDriver(t *testing.T) {
"versionTablename": "Version",
}

driver = storage.CreateCacheDatabaseDriver(conf.Storage.StorageConfig, log)
driver, err = storage.CreateCacheDatabaseDriver(conf.Storage.StorageConfig, log)
So(err, ShouldBeNil)
So(driver, ShouldBeNil)

conf.Storage.CacheDriver = map[string]interface{}{
Expand All @@ -199,7 +203,8 @@ func TestCreateCacheDatabaseDriver(t *testing.T) {
"versionTablename": "Version",
}

driver = storage.CreateCacheDatabaseDriver(conf.Storage.StorageConfig, log)
driver, err = storage.CreateCacheDatabaseDriver(conf.Storage.StorageConfig, log)
So(err, ShouldBeNil)
So(driver, ShouldBeNil)
})
}
Expand Down Expand Up @@ -2547,15 +2552,15 @@ func TestNewRelyingPartyOIDC(t *testing.T) {
}

Convey("provider not found in config", func() {
So(func() { _ = api.NewRelyingPartyOIDC(conf, "notDex") }, ShouldPanic)
So(func() { _ = api.NewRelyingPartyOIDC(conf, "notDex", log.NewLogger("debug", "")) }, ShouldPanic)
})

Convey("key path not found on disk", func() {
oidcProviderCfg := conf.HTTP.Auth.OpenID.Providers["oidc"]
oidcProviderCfg.KeyPath = "path/to/file"
conf.HTTP.Auth.OpenID.Providers["oidc"] = oidcProviderCfg

So(func() { _ = api.NewRelyingPartyOIDC(conf, "oidc") }, ShouldPanic)
So(func() { _ = api.NewRelyingPartyOIDC(conf, "oidc", log.NewLogger("debug", "")) }, ShouldPanic)
})

Convey("https callback", func() {
Expand All @@ -2564,7 +2569,7 @@ func TestNewRelyingPartyOIDC(t *testing.T) {
Key: ServerKey,
}

rp := api.NewRelyingPartyOIDC(conf, "oidc")
rp := api.NewRelyingPartyOIDC(conf, "oidc", log.NewLogger("debug", ""))
So(rp, ShouldNotBeNil)
})

Expand All @@ -2573,7 +2578,7 @@ func TestNewRelyingPartyOIDC(t *testing.T) {
oidcProvider.ClientSecret = ""
conf.HTTP.Auth.OpenID.Providers["oidc"] = oidcProvider

rp := api.NewRelyingPartyOIDC(conf, "oidc")
rp := api.NewRelyingPartyOIDC(conf, "oidc", log.NewLogger("debug", ""))
So(rp, ShouldNotBeNil)
})

Expand All @@ -2582,7 +2587,7 @@ func TestNewRelyingPartyOIDC(t *testing.T) {
oidcProvider.Issuer = ""
conf.HTTP.Auth.OpenID.Providers["oidc"] = oidcProvider

So(func() { _ = api.NewRelyingPartyOIDC(conf, "oidc") }, ShouldPanic)
So(func() { _ = api.NewRelyingPartyOIDC(conf, "oidc", log.NewLogger("debug", "")) }, ShouldPanic)
})
})
}
Expand Down
6 changes: 3 additions & 3 deletions pkg/cli/config_cmd.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ func NewConfigCommand() *cobra.Command {
RunE: func(cmd *cobra.Command, args []string) error {
home, err := os.UserHomeDir()
if err != nil {
panic(err)
return err
}

configPath := path.Join(home + "/.zot")
Expand Down Expand Up @@ -112,7 +112,7 @@ func NewConfigAddCommand() *cobra.Command {
RunE: func(cmd *cobra.Command, args []string) error {
home, err := os.UserHomeDir()
if err != nil {
panic(err)
return err
}

configPath := path.Join(home + "/.zot")
Expand Down Expand Up @@ -142,7 +142,7 @@ func NewConfigRemoveCommand() *cobra.Command {
RunE: func(cmd *cobra.Command, args []string) error {
home, err := os.UserHomeDir()
if err != nil {
panic(err)
return err
}

configPath := path.Join(home + "/.zot")
Expand Down
6 changes: 2 additions & 4 deletions pkg/cli/config_reloader.go
Original file line number Diff line number Diff line change
Expand Up @@ -105,15 +105,13 @@ func (hr *HotReloader) Start() context.Context {
}
// watch for errors
case err := <-hr.watcher.Errors:
log.Error().Err(err).Str("config", hr.filePath).Msg("fsnotfy error while watching config")
panic(err)
log.Panic().Err(err).Str("config", hr.filePath).Msg("fsnotfy error while watching config")
}
}
}()

if err := hr.watcher.Add(hr.filePath); err != nil {
log.Error().Err(err).Str("config", hr.filePath).Msg("error adding config file to FsNotify watcher")
panic(err)
log.Panic().Err(err).Str("config", hr.filePath).Msg("error adding config file to FsNotify watcher")
}

<-done
Expand Down
2 changes: 1 addition & 1 deletion pkg/cli/cve_cmd.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ func NewCveCommand(searchService SearchService) *cobra.Command {
RunE: func(cmd *cobra.Command, args []string) error {
home, err := os.UserHomeDir()
if err != nil {
panic(err)
return err
}

configPath := path.Join(home + "/.zot")
Expand Down
2 changes: 1 addition & 1 deletion pkg/cli/image_cmd.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ func NewImageCommand(searchService SearchService) *cobra.Command {
RunE: func(cmd *cobra.Command, args []string) error {
home, err := os.UserHomeDir()
if err != nil {
panic(err)
return err
}

configPath := path.Join(home + "/.zot")
Expand Down
2 changes: 1 addition & 1 deletion pkg/cli/repo_cmd.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ func NewRepoCommand(searchService SearchService) *cobra.Command {
RunE: func(cmd *cobra.Command, args []string) error {
home, err := os.UserHomeDir()
if err != nil {
panic(err)
return err
}

configPath := path.Join(home + "/.zot")
Expand Down
Loading

0 comments on commit 064d471

Please sign in to comment.