Fix loading the iRODS environment file into the manager

Move manager.Load() to the correct place so that the manager
configuration sequence works again and the IRODS_PASSWORD environment
variable is only required when an auth file is not present.

docker-compose.yml

@@ -27,7 +27,7 @@ services:
                 "--irods-env", "/app/config/app_irods_environment.json",
                 "--log-level", "trace"]
       environment:
-        IRODS_PASSWORD: "irods"
+        IRODS_PASSWORD: "irods"  # Required when the app auth file is not present
       ports:
         - "3333:3333"
       volumes:

server/irods.go

@@ -85,76 +85,63 @@ func InitIRODS(logger zerolog.Logger, manager *icommands.ICommandsEnvironmentMan
 // This function creates a manager and sets the iRODS environment file path from the
 // shell environment. If an iRODS auth file is present, the password is read from it.
 // Otherwise, the password is read from the shell environment.
-func NewICommandsEnvironmentManager() (manager *icommands.ICommandsEnvironmentManager, err error) {
+func NewICommandsEnvironmentManager(logger zerolog.Logger,
+	iRODSEnvFilePath string) (manager *icommands.ICommandsEnvironmentManager, err error) {
+	if iRODSEnvFilePath == "" {
+		return nil, fmt.Errorf("iRODS environment file path was empty: %w",
+			ErrInvalidArgument)
+	}
+
+	// manager.Load() below will succeed even if the iRODS environment file does not
+	// exist, but we absolutely don't want that behaviour here.
+	var fileInfo os.FileInfo
+	if fileInfo, err = os.Stat(iRODSEnvFilePath); err != nil && os.IsNotExist(err) {
+		return nil, err
+	}
+	if fileInfo.IsDir() {
+		return nil, fmt.Errorf("iRODS environment file is a directory: %w",
+			ErrInvalidArgument)
+	}
 	if manager, err = icommands.CreateIcommandsEnvironmentManager(); err != nil {
 		return nil, err
 	}
-	if err = manager.SetEnvironmentFilePath(IRODSEnvFilePath()); err != nil {
+	if err = manager.SetEnvironmentFilePath(iRODSEnvFilePath); err != nil {
 		return nil, err
 	}
+	if err = manager.Load(os.Getpid()); err != nil {
+		return nil, err
+	}
+
+	logger.Info().
+		Str("path", iRODSEnvFilePath).
+		Msg("Loaded iRODS environment file")
 
 	authFilePath := manager.GetPasswordFilePath()
 
 	// An existing auth file takes precedence over the environment variable
 	if _, err = os.Stat(authFilePath); err != nil && os.IsNotExist(err) {
-		envPassword, ok := os.LookupEnv(IRODSPasswordEnvVar)
+		password, ok := os.LookupEnv(IRODSPasswordEnvVar)
 		if !ok {
-			return nil, fmt.Errorf("%s environment variable was not set: %w",
-				IRODSPasswordEnvVar, ErrMissingArgument)
+			return nil, fmt.Errorf("iRODS auth file '%s' was not present "+
+				"and the '%s' environment variable needed to create it was not set: %w",
+				authFilePath, IRODSPasswordEnvVar, ErrMissingArgument)
 		}
-		if envPassword == "" {
-			return nil, fmt.Errorf("%s environment variable was empty: %w",
-				IRODSPasswordEnvVar, ErrInvalidArgument)
+		if password == "" {
+			return nil, fmt.Errorf("iRODS auth file '%s' was not present "+
+				"and the '%s' environment variable needed to set it was empty: %w",
+				authFilePath, IRODSPasswordEnvVar, ErrInvalidArgument)
 		}
 
-		manager.Password = envPassword
-	} else {
-		filePassword, perr := icommands.DecodePasswordFile(authFilePath, os.Getuid())
-		if perr != nil {
-			return nil, perr
-		}
-
-		manager.Password = filePassword
+		manager.Password = password // manager.Password is propagated to the iRODS account
 	}
 
-	// manager.Password is propagated to the iRODS account
-
 	return manager, nil
 }
 
 // NewIRODSAccount returns an iRODS account instance using the iRODS environment for
 // configuration. The environment file path is obtained from the manager.
 func NewIRODSAccount(logger zerolog.Logger,
 	manager *icommands.ICommandsEnvironmentManager) (account *types.IRODSAccount, err error) { // NRV
-	// manager.Load() below will succeed even if the iRODS environment file does not
-	// exist, but we absolutely don't want that behaviour here.
-
-	envFilePath := manager.GetEnvironmentFilePath()
-
-	var fileInfo os.FileInfo
-	if fileInfo, err = os.Stat(envFilePath); err != nil && os.IsNotExist(err) {
-		logger.Err(err).
-			Str("path", envFilePath).
-			Msg("iRODS environment file does not exist")
-		return nil, err
-	}
-	if fileInfo.IsDir() {
-		err = fmt.Errorf("iRODS environment file is a directory: %w", ErrInvalidArgument)
-		logger.Err(err).
-			Str("path", envFilePath).
-			Msg("iRODS environment file is a directory")
-		return nil, err
-	}
-
-	if err = manager.Load(os.Getpid()); err != nil {
-		logger.Err(err).Msg("iRODS environment manager failed to load an environment")
-		return nil, err
-	}
-
-	logger.Info().
-		Str("path", envFilePath).
-		Msg("Loaded iRODS environment file")
-
 	if account, err = manager.ToIRODSAccount(); err != nil {
 		logger.Err(err).Msg("Failed to obtain an iRODS account instance")
 		return nil, err

server/server.go

@@ -37,7 +37,6 @@ import (
 	"github.com/cyverse/go-irodsclient/fs"
 	"github.com/cyverse/go-irodsclient/icommands"
 	"github.com/cyverse/go-irodsclient/irods/types"
-	"github.com/cyverse/go-irodsclient/irods/util"
 	"github.com/microcosm-cc/bluemonday"
 	"github.com/rs/zerolog"
 )
@@ -129,12 +128,31 @@ func NewSqyrrlServer(logger zerolog.Logger, config Config) (server *SqyrrlServer
 		return nil, err
 	}
 
+	var cwd string
+	cwd, err = os.Getwd()
+	if err != nil {
+		return nil, err
+	}
+
 	subLogger := logger.With().
 		Str("hostname", hostname).
 		Str("component", "server").Logger()
 
 	var manager *icommands.ICommandsEnvironmentManager
-	if manager, err = NewICommandsEnvironmentManager(); err != nil {
+	if config.EnvFilePath == "" {
+		config.EnvFilePath = IRODSEnvFilePath()
+	}
+
+	logger.Debug().
+		Str("host", config.Host).
+		Str("port", config.Port).
+		Str("cert_file", config.CertFilePath).
+		Str("key_file", config.KeyFilePath).
+		Str("irods_env", config.EnvFilePath).
+		Str("cwd", cwd).
+		Dur("index_interval", config.IndexInterval).Msg("Server configured")
+
+	if manager, err = NewICommandsEnvironmentManager(subLogger, config.EnvFilePath); err != nil {
 		logger.Err(err).Msg("Failed to create an iRODS environment manager")
 		return nil, err
 	}
@@ -170,12 +188,6 @@ func NewSqyrrlServer(logger zerolog.Logger, config Config) (server *SqyrrlServer
 		return nil, err
 	}
 
-	var cwd string
-	cwd, err = os.Getwd()
-	if err != nil {
-		return nil, err
-	}
-
 	server.setUpSignalHandler()
 	server.addRoutes(mux)
 
@@ -333,40 +345,23 @@ func (server *SqyrrlServer) waitAndShutdown() (err error) { // NRV
 
 func ConfigureAndStart(logger zerolog.Logger, config Config) error {
 	if config.Host == "" {
-		// return errors.New("missing host component of address to listen on")
 		return fmt.Errorf("server config %w: address", ErrMissingArgument)
 	}
 	if config.Port == "" {
-		// return errors.New("missing port component of address to listen on")
 		return fmt.Errorf("server config %w: port", ErrMissingArgument)
 	}
 	if config.CertFilePath == "" {
-		// return errors.New("missing certificate file path")
 		return fmt.Errorf("server config %w: certificate file path", ErrMissingArgument)
 	}
 	if config.KeyFilePath == "" {
-		// return errors.New("missing key file path")
 		return fmt.Errorf("server config %w: key file path", ErrMissingArgument)
 	}
-	if config.EnvFilePath == "" {
-		// return errors.New("missing iRODS environment file path")
-		return fmt.Errorf("server config %w: iRODS environment file path", ErrMissingArgument)
-	}
 	if !(config.IndexInterval > 0) {
-		// return errors.New("missing index interval")
 		return fmt.Errorf("server config %w: index interval", ErrMissingArgument)
 	}
 
-	envFilePath, err := util.ExpandHomeDir(config.EnvFilePath)
-	if err != nil {
-		logger.Err(err).Str("path", config.EnvFilePath).
-			Msg("Failed to expand the iRODS environment file path")
-		return err
-	}
-	config.EnvFilePath = envFilePath
-
 	var server *SqyrrlServer
-	server, err = NewSqyrrlServer(logger, config)
+	server, err := NewSqyrrlServer(logger, config)
 	if err != nil {
 		return err
 	}

server/server_suite_test.go

@@ -72,7 +72,7 @@ var _ = BeforeSuite(func() {
 		Expect(err).NotTo(HaveOccurred())
 	}
 
-	manager, err := server.NewICommandsEnvironmentManager()
+	manager, err := server.NewICommandsEnvironmentManager(suiteLogger, iRODSEnvFilePath)
 	Expect(err).NotTo(HaveOccurred())
 	Expect(manager.GetEnvironmentFilePath()).To(Equal(iRODSEnvFilePath))
 	Expect(manager.Password).To(Equal(iRODSPassword))

server/server_test.go

@@ -28,6 +28,7 @@ var _ = Describe("Server startup and shutdown", func() {
 			Port:          port,
 			CertFilePath:  filepath.Join(configDir, "localhost.crt"),
 			KeyFilePath:   filepath.Join(configDir, "localhost.key"),
+			EnvFilePath:   filepath.Join(configDir, "test_irods_environment.json"),
 			IndexInterval: server.DefaultIndexInterval,
 		}
 	})