From 6f7ae55bbae3b3ba45caf48ba2262076c93ed22f Mon Sep 17 00:00:00 2001 From: Sylvie Date: Mon, 1 Jul 2024 17:25:05 -0500 Subject: [PATCH 01/12] Start work on unzipping a password protected file; Update sftp.go to initialize correctly Co-Authored-By: pluckyswan <96704946+pluckyswan@users.noreply.github.com> Co-Authored-By: Samuel Aquino Co-Authored-By: jcrichlake <145698165+jcrichlake@users.noreply.github.com> Co-Authored-By: jherrflexion <118225331+jherrflexion@users.noreply.github.com> --- src/go.mod | 1 + src/go.sum | 2 ++ src/sftp/sftp.go | 35 ++++++++++++++++++++++++++++---- src/sftp/sftp_test.go | 8 ++++---- src/usecases/read_and_send.go | 38 +++++++++++++++++++++++++++++++++++ 5 files changed, 76 insertions(+), 8 deletions(-) diff --git a/src/go.mod b/src/go.mod index 34797bfd..d043ddb7 100644 --- a/src/go.mod +++ b/src/go.mod @@ -27,6 +27,7 @@ require ( github.com/pkg/browser v0.0.0-20240102092130-5ac0b6a4141c // indirect github.com/pmezard/go-difflib v1.0.0 // indirect github.com/stretchr/objx v0.5.2 // indirect + github.com/yeka/zip v0.0.0-20231116150916-03d6312748a9 // indirect golang.org/x/net v0.26.0 // indirect golang.org/x/sys v0.21.0 // indirect golang.org/x/text v0.16.0 // indirect diff --git a/src/go.sum b/src/go.sum index b1666d2a..6b9fe3b0 100644 --- a/src/go.sum +++ b/src/go.sum @@ -51,6 +51,8 @@ github.com/stretchr/testify v1.7.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/ github.com/stretchr/testify v1.8.0/go.mod h1:yNjHg4UonilssWZ8iaSj1OCr/vHnekPRkoO+kdMU+MU= github.com/stretchr/testify v1.9.0 h1:HtqpIVDClZ4nwg75+f6Lvsy/wHu+3BoSGCbBAcpTsTg= github.com/stretchr/testify v1.9.0/go.mod h1:r2ic/lqez/lEtzL7wO/rwa5dbSLXVDPFyf8C91i36aY= +github.com/yeka/zip v0.0.0-20231116150916-03d6312748a9 h1:K8gF0eekWPEX+57l30ixxzGhHH/qscI3JCnuhbN6V4M= +github.com/yeka/zip v0.0.0-20231116150916-03d6312748a9/go.mod h1:9BnoKCcgJ/+SLhfAXj15352hTOuVmG5Gzo8xNRINfqI= github.com/yuin/goldmark v1.4.13/go.mod h1:6yULJ656Px+3vBD8DxQVa3kxgyrAnzto9xy5taEt/CY= golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w= golang.org/x/crypto v0.0.0-20210921155107-089bfa567519/go.mod h1:GvvjBRRGRdwPK5ydBHafDWAxML/pGHZbMvKqRZ5+Abc= diff --git a/src/sftp/sftp.go b/src/sftp/sftp.go index a516c3e5..eedee3ce 100644 --- a/src/sftp/sftp.go +++ b/src/sftp/sftp.go @@ -9,13 +9,14 @@ import ( "io" "log/slog" "os" + "strings" ) type SftpHandler struct { sshClient *ssh.Client sftpClient SftpClient blobHandler usecases.BlobHandler - IoWrapper IoWrapper + ioClient IoClient } type SftpClient interface { @@ -80,10 +81,13 @@ func NewSftpHandler() (*SftpHandler, error) { return nil, err } + ioWrapper := IoWrapper{} + return &SftpHandler{ sshClient: sshClient, sftpClient: sftpClient, blobHandler: blobHandler, + ioClient: &ioWrapper, }, nil } @@ -168,7 +172,8 @@ func (receiver *SftpHandler) copySingleFile(fileInfo os.FileInfo, index int, dir return } - fileBytes, err := receiver.IoWrapper.ReadBytesFromFile(file) + slog.Info("file opened", slog.String("name", fileInfo.Name()), slog.Any("file", file)) + fileBytes, err := receiver.ioClient.ReadBytesFromFile(file) if err != nil { slog.Error("Failed to read file", slog.Any("error", err)) return @@ -179,12 +184,34 @@ func (receiver *SftpHandler) copySingleFile(fileInfo os.FileInfo, index int, dir if err != nil { slog.Error("Failed to upload file", slog.Any("error", err)) } + + slog.Info("About to consider whether this is a zip", slog.String("file name", fileInfo.Name())) + if strings.Contains(fileInfo.Name(), ".zip") { + // write file to local filesystem + err = os.WriteFile(fileInfo.Name(), fileBytes, 0644) // permissions = owner read/write, group read, other read + if err != nil { + slog.Error("Failed to write file", slog.Any("error", err), slog.String("name", fileInfo.Name())) + return + } + + _ = usecases.UnzipProtected(fileInfo.Name()) + + //delete file from local filesystem + err = os.Remove(fileInfo.Name()) + if err != nil { + slog.Error("Failed to remove file", slog.Any("error", err), slog.String("name", fileInfo.Name())) + } + + } } -type IoWrapper interface { +type IoClient interface { ReadBytesFromFile(file *sftp.File) ([]byte, error) } -func (receiver *SftpHandler) ReadBytesFromFile(file *sftp.File) ([]byte, error) { +type IoWrapper struct { +} + +func (receiver *IoWrapper) ReadBytesFromFile(file *sftp.File) ([]byte, error) { return io.ReadAll(file) } diff --git a/src/sftp/sftp_test.go b/src/sftp/sftp_test.go index b64dbe2e..9f07c454 100644 --- a/src/sftp/sftp_test.go +++ b/src/sftp/sftp_test.go @@ -104,7 +104,7 @@ func Test_CopyFiles_happyPath(t *testing.T) { mockBlobHandler.On("UploadFile", mock.Anything, mock.Anything).Return(nil) - sftpHandler := SftpHandler{sftpClient: mockSftpClient, blobHandler: mockBlobHandler, IoWrapper: mockIoWrapper} + sftpHandler := SftpHandler{sftpClient: mockSftpClient, blobHandler: mockBlobHandler, ioClient: mockIoWrapper} sftpHandler.CopyFiles() @@ -162,7 +162,7 @@ func Test_copySingleFile_happyPath(t *testing.T) { mockBlobHandler.On("UploadFile", mock.Anything, mock.Anything).Return(nil) - sftpHandler := SftpHandler{sftpClient: mockSftpClient, blobHandler: mockBlobHandler, IoWrapper: mockIoWrapper} + sftpHandler := SftpHandler{sftpClient: mockSftpClient, blobHandler: mockBlobHandler, ioClient: mockIoWrapper} sftpHandler.copySingleFile(fileInfo, 1, fileDirectory) mockBlobHandler.AssertCalled(t, "UploadFile", mock.Anything, mock.Anything) @@ -242,7 +242,7 @@ func Test_copySingleFile_failedToReadFile(t *testing.T) { filePath := filepath.Join(fileDirectory, "copy_file_test.txt") fileInfo, _ := os.Stat(filePath) - sftpHandler := SftpHandler{sftpClient: mockSftpClient, IoWrapper: mockIoWrapper} + sftpHandler := SftpHandler{sftpClient: mockSftpClient, ioClient: mockIoWrapper} sftpHandler.copySingleFile(fileInfo, 1, fileDirectory) mockIoWrapper.AssertCalled(t, "ReadBytesFromFile", mock.Anything) @@ -275,7 +275,7 @@ func Test_copySingleFile_failToUploadFile(t *testing.T) { mockBlobHandler := &mocks.MockBlobHandler{} mockBlobHandler.On("UploadFile", mock.Anything, mock.Anything).Return(errors.New("error")) - sftpHandler := SftpHandler{sftpClient: mockSftpClient, blobHandler: mockBlobHandler, IoWrapper: mockIoWrapper} + sftpHandler := SftpHandler{sftpClient: mockSftpClient, blobHandler: mockBlobHandler, ioClient: mockIoWrapper} sftpHandler.copySingleFile(fileInfo, 1, fileDirectory) mockBlobHandler.AssertCalled(t, "UploadFile", mock.Anything, mock.Anything) diff --git a/src/usecases/read_and_send.go b/src/usecases/read_and_send.go index 2b7906b1..6426be10 100644 --- a/src/usecases/read_and_send.go +++ b/src/usecases/read_and_send.go @@ -3,6 +3,8 @@ package usecases import ( "github.com/CDCgov/reportstream-sftp-ingestion/senders" "github.com/CDCgov/reportstream-sftp-ingestion/storage" + "github.com/yeka/zip" + "io" "log/slog" "os" "strings" @@ -93,3 +95,39 @@ func (receiver *ReadAndSendUsecase) moveFile(sourceUrl string, newFolderName str slog.Error("Failed to move file after processing", slog.Any("error", err)) } } + +// TODO - this probably belongs in a different file +func UnzipProtected(zipFilePath string) error { + slog.Info("Called unzip protected") + zipReader, err := zip.OpenReader(zipFilePath) + + if err != nil { + slog.Error("Failed to open zip reader", slog.Any("error", err)) + return err + } + defer zipReader.Close() + + for _, f := range zipReader.File { + slog.Info("inside of zip Reader loop") + // TODO - should we warn or error if not encrypted? This would vary per customer + if f.IsEncrypted() { + f.SetPassword("test123") + } + slog.Info("setting password") + fileReader, err := f.Open() + if err != nil { + slog.Error("Failed to open file", slog.Any("error", err)) + } + defer fileReader.Close() + + slog.Info("file opened", slog.Any("file", f)) + buf, err := io.ReadAll(fileReader) + + slog.Info(string(buf)) + + if err != nil { + slog.Error("Failed to read file", slog.Any("error", err)) + } + } + return nil +} From cadf18ca95bf444a1137460dac7b7cf71a5830c4 Mon Sep 17 00:00:00 2001 From: Sylvie Date: Tue, 2 Jul 2024 14:13:40 -0500 Subject: [PATCH 02/12] Moved zip to its own package; added env var for CA DPH password; upload unzipped files to import; use constants for repeated strings; a lil cleanup and refactoring Co-Authored-By: pluckyswan <96704946+pluckyswan@users.noreply.github.com> Co-Authored-By: Samuel Aquino Co-Authored-By: jcrichlake <145698165+jcrichlake@users.noreply.github.com> Co-Authored-By: jherrflexion <118225331+jherrflexion@users.noreply.github.com> --- docker-compose.yml | 5 +- mock_credentials/mock_ca_dph_zip_password.txt | 1 + operations/template/app.tf | 1 + operations/template/event.tf | 2 +- operations/template/key.tf | 12 +++ src/cmd/main.go | 9 +- src/go.mod | 2 +- src/orchestration/queue.go | 33 +++--- src/secrets/azure_secret_getter.go | 7 +- src/{utils => secrets}/credential_getter.go | 7 +- .../credential_getter_test.go | 2 +- src/secrets/local_credential_getter.go | 6 +- src/senders/report_stream_sender.go | 14 +-- src/senders/report_stream_sender_test.go | 3 +- src/sftp/sftp.go | 47 ++++---- src/sftp/sftp_test.go | 11 +- src/storage/azure.go | 12 +-- src/usecases/read_and_send.go | 57 ++-------- src/utils/constants.go | 20 ++++ src/zip/zip.go | 102 ++++++++++++++++++ 20 files changed, 231 insertions(+), 122 deletions(-) create mode 100644 mock_credentials/mock_ca_dph_zip_password.txt rename src/{utils => secrets}/credential_getter.go (81%) rename src/{utils => secrets}/credential_getter_test.go (98%) create mode 100644 src/zip/zip.go diff --git a/docker-compose.yml b/docker-compose.yml index c2b29753..61dc83b4 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -11,10 +11,11 @@ services: FLEXION_CLIENT_NAME: flexion.simulated-hospital QUEUE_MAX_DELIVERY_ATTEMPTS: 5 SFTP_USER: ti_user - SFTP_PASSWORD: ti_password #TODO - update #pragma: allowlist secret - SFTP_KEY_NAME: sftp_server_user_id_rsa.pem #TODO - update #pragma: allowlist secret + SFTP_PASSWORD: ti_password #pragma: allowlist secret + SFTP_KEY_NAME: sftp_server_user_id_rsa.pem #pragma: allowlist secret SFTP_SERVER_ADDRESS: host.docker.internal:2223 # no http since this is sftp SFTP_SERVER_PUBLIC_KEY_NAME: ssh_host_rsa_key.pub + CA_DPH_ZIP_PASSWORD_NAME: mock_ca_dph_zip_password.txt #pragma: allowlist secret volumes: # map to Azurite data objects to the build directory - ./localdata/data/reportstream:/localdata diff --git a/mock_credentials/mock_ca_dph_zip_password.txt b/mock_credentials/mock_ca_dph_zip_password.txt new file mode 100644 index 00000000..5271a526 --- /dev/null +++ b/mock_credentials/mock_ca_dph_zip_password.txt @@ -0,0 +1 @@ +test123 diff --git a/operations/template/app.tf b/operations/template/app.tf index e2f0d248..79f08212 100644 --- a/operations/template/app.tf +++ b/operations/template/app.tf @@ -71,6 +71,7 @@ resource "azurerm_linux_web_app" "sftp" { AZURE_KEY_VAULT_URI = azurerm_key_vault.key_storage.vault_uri FLEXION_CLIENT_NAME = "flexion.simulated-lab" QUEUE_MAX_DELIVERY_ATTEMPTS = azurerm_eventgrid_system_topic_event_subscription.topic_sub.retry_policy.0.max_delivery_attempts # making the Azure container <-> queue retry count be in sync with the queue <-> application retry count.. + CA_DPH_ZIP_PASSWORD_NAME = azurerm_key_vault_secret.ca_dph_zip_password.name } identity { diff --git a/operations/template/event.tf b/operations/template/event.tf index c76fe579..91872c92 100644 --- a/operations/template/event.tf +++ b/operations/template/event.tf @@ -28,7 +28,7 @@ resource "azurerm_eventgrid_system_topic_event_subscription" "topic_sub" { advanced_filter { string_contains { key = "subject" - values = ["import"] + values = ["/import/"] } } diff --git a/operations/template/key.tf b/operations/template/key.tf index 8d536cac..1384dc38 100644 --- a/operations/template/key.tf +++ b/operations/template/key.tf @@ -45,3 +45,15 @@ resource "azurerm_key_vault_secret" "mock_public_health_lab_private_key" { } depends_on = [azurerm_key_vault_access_policy.allow_github_deployer] //wait for the permission that allows our deployer to write the secret } + +resource "azurerm_key_vault_secret" "ca_dph_zip_password" { + name = "ca-dph-zip-password-${var.environment}" + value = "dogcow" + + key_vault_id = azurerm_key_vault.key_storage.id + + lifecycle { + ignore_changes = [value] + } + depends_on = [azurerm_key_vault_access_policy.allow_github_deployer] //wait for the permission that allows our deployer to write the secret +} diff --git a/src/cmd/main.go b/src/cmd/main.go index 4500f5ec..c5c1bcba 100644 --- a/src/cmd/main.go +++ b/src/cmd/main.go @@ -3,6 +3,7 @@ package main import ( "github.com/CDCgov/reportstream-sftp-ingestion/orchestration" "github.com/CDCgov/reportstream-sftp-ingestion/sftp" + "github.com/CDCgov/reportstream-sftp-ingestion/utils" "io" "log/slog" "net/http" @@ -19,13 +20,13 @@ func main() { queueHandler, err := orchestration.NewQueueHandler() if err != nil { - slog.Warn("Failed to create queueHandler", slog.Any("error", err)) + slog.Warn("Failed to create queueHandler", slog.Any(utils.ErrorKey, err)) } // TODO - move calls to SFTP into whatever timer/trigger we set up later sftpHandler, err := sftp.NewSftpHandler() if err != nil { - slog.Error("ope, failed to create sftp handler", slog.Any("error", err)) + slog.Error("ope, failed to create sftp handler", slog.Any(utils.ErrorKey, err)) // Don't return, we want to let things keep going for now } defer sftpHandler.Close() @@ -58,12 +59,12 @@ func setupHealthCheck() { _, err := io.WriteString(response, "Operational") if err != nil { - slog.Error("Failed to respond to health check", slog.Any("error", err)) + slog.Error("Failed to respond to health check", slog.Any(utils.ErrorKey, err)) } }) err := http.ListenAndServe(":8080", nil) if err != nil { - slog.Error("Failed to start health check", slog.Any("error", err)) + slog.Error("Failed to start health check", slog.Any(utils.ErrorKey, err)) } } diff --git a/src/go.mod b/src/go.mod index d043ddb7..ac73394a 100644 --- a/src/go.mod +++ b/src/go.mod @@ -13,6 +13,7 @@ require ( github.com/google/uuid v1.6.0 github.com/pkg/sftp v1.13.6 github.com/stretchr/testify v1.9.0 + github.com/yeka/zip v0.0.0-20231116150916-03d6312748a9 golang.org/x/crypto v0.24.0 ) @@ -27,7 +28,6 @@ require ( github.com/pkg/browser v0.0.0-20240102092130-5ac0b6a4141c // indirect github.com/pmezard/go-difflib v1.0.0 // indirect github.com/stretchr/objx v0.5.2 // indirect - github.com/yeka/zip v0.0.0-20231116150916-03d6312748a9 // indirect golang.org/x/net v0.26.0 // indirect golang.org/x/sys v0.21.0 // indirect golang.org/x/text v0.16.0 // indirect diff --git a/src/orchestration/queue.go b/src/orchestration/queue.go index 332587d7..a4f8971c 100644 --- a/src/orchestration/queue.go +++ b/src/orchestration/queue.go @@ -8,6 +8,7 @@ import ( "github.com/Azure/azure-sdk-for-go/sdk/messaging/eventgrid/azeventgrid" "github.com/Azure/azure-sdk-for-go/sdk/storage/azqueue" "github.com/CDCgov/reportstream-sftp-ingestion/usecases" + "github.com/CDCgov/reportstream-sftp-ingestion/utils" "log/slog" "os" "strconv" @@ -31,20 +32,20 @@ func NewQueueHandler() (QueueHandler, error) { client, err := azqueue.NewQueueClientFromConnectionString(azureQueueConnectionString, "blob-message-queue", nil) if err != nil { - slog.Error("Unable to create Azure Queue Client for primary queue", slog.Any("error", err)) + slog.Error("Unable to create Azure Queue Client for primary queue", slog.Any(utils.ErrorKey, err)) return QueueHandler{}, err } dlqClient, err := azqueue.NewQueueClientFromConnectionString(azureQueueConnectionString, "blob-message-dead-letter-queue", nil) if err != nil { - slog.Error("Unable to create Azure Queue Client for dead letter queue", slog.Any("error", err)) + slog.Error("Unable to create Azure Queue Client for dead letter queue", slog.Any(utils.ErrorKey, err)) return QueueHandler{}, err } usecase, err := usecases.NewReadAndSendUsecase() if err != nil { - slog.Error("Unable to create Usecase", slog.Any("error", err)) + slog.Error("Unable to create Usecase", slog.Any(utils.ErrorKey, err)) return QueueHandler{}, err } @@ -54,7 +55,7 @@ func NewQueueHandler() (QueueHandler, error) { func getUrlFromMessage(messageText string) (string, error) { eventBytes, err := base64.StdEncoding.DecodeString(messageText) if err != nil { - slog.Error("Failed to decode message text", slog.Any("error", err)) + slog.Error("Failed to decode message text", slog.Any(utils.ErrorKey, err)) return "", err } @@ -62,7 +63,7 @@ func getUrlFromMessage(messageText string) (string, error) { var event azeventgrid.Event err = event.UnmarshalJSON(eventBytes) if err != nil { - slog.Error("Failed to unmarshal event", slog.Any("error", err)) + slog.Error("Failed to unmarshal event", slog.Any(utils.ErrorKey, err)) return "", err } @@ -91,7 +92,7 @@ func (receiver QueueHandler) deleteMessage(message azqueue.DequeuedMessage) erro deleteResponse, err := receiver.queueClient.DeleteMessage(context.Background(), messageId, popReceipt, nil) if err != nil { - slog.Error("Unable to delete message", slog.Any("error", err)) + slog.Error("Unable to delete message", slog.Any(utils.ErrorKey, err)) return err } @@ -112,20 +113,20 @@ func (receiver QueueHandler) handleMessage(message azqueue.DequeuedMessage) erro sourceUrl, err := getUrlFromMessage(*message.MessageText) if err != nil { - slog.Error("Failed to get the file URL", slog.Any("error", err)) + slog.Error("Failed to get the file URL", slog.Any(utils.ErrorKey, err)) return err } err = receiver.usecase.ReadAndSend(sourceUrl) if err != nil { - slog.Warn("Failed to read/send file", slog.Any("error", err)) + slog.Warn("Failed to read/send file", slog.Any(utils.ErrorKey, err)) } else { // Only delete message if file successfully sent to ReportStream // (or if there's a known non-transient error and we've moved the file to `failure`) err = receiver.deleteMessage(message) if err != nil { - slog.Warn("Failed to delete message", slog.Any("error", err)) + slog.Warn("Failed to delete message", slog.Any(utils.ErrorKey, err)) return err } } @@ -141,14 +142,14 @@ func (receiver QueueHandler) overDeliveryThreshold(message azqueue.DequeuedMessa if err != nil { maxDeliveryCount = 5 - slog.Warn("Failed to parse QUEUE_MAX_DELIVERY_ATTEMPTS, defaulting to 5", slog.Any("error", err)) + slog.Warn("Failed to parse QUEUE_MAX_DELIVERY_ATTEMPTS, defaulting to 5", slog.Any(utils.ErrorKey, err)) } if *message.DequeueCount > maxDeliveryCount { slog.Error("Message reached maximum number of delivery attempts", slog.Any("message", message)) err := receiver.deadLetter(message) if err != nil { - slog.Error("Failed to move message to the DLQ", slog.Any("message", message), slog.Any("error", err)) + slog.Error("Failed to move message to the DLQ", slog.Any("message", message), slog.Any(utils.ErrorKey, err)) } return true } @@ -161,13 +162,13 @@ func (receiver QueueHandler) deadLetter(message azqueue.DequeuedMessage) error { opts := &azqueue.EnqueueMessageOptions{TimeToLive: to.Ptr(int32(-1))} _, err := receiver.deadLetterQueueClient.EnqueueMessage(context.Background(), *message.MessageText, opts) if err != nil { - slog.Error("Failed to add the message to the DLQ", slog.Any("error", err)) + slog.Error("Failed to add the message to the DLQ", slog.Any(utils.ErrorKey, err)) return err } err = receiver.deleteMessage(message) if err != nil { - slog.Error("Failed to delete the message to the original queue after adding it to the DLQ", slog.Any("error", err)) + slog.Error("Failed to delete the message to the original queue after adding it to the DLQ", slog.Any(utils.ErrorKey, err)) return err } @@ -180,7 +181,7 @@ func (receiver QueueHandler) ListenToQueue() { for { err := receiver.receiveQueue() if err != nil { - slog.Error("Failed to receive message", slog.Any("error", err)) + slog.Error("Failed to receive message", slog.Any(utils.ErrorKey, err)) } time.Sleep(10 * time.Second) } @@ -192,7 +193,7 @@ func (receiver QueueHandler) receiveQueue() error { messageResponse, err := receiver.queueClient.DequeueMessage(context.Background(), nil) if err != nil { - slog.Error("Unable to dequeue messages", slog.Any("error", err)) + slog.Error("Unable to dequeue messages", slog.Any(utils.ErrorKey, err)) return err } @@ -201,7 +202,7 @@ func (receiver QueueHandler) receiveQueue() error { go func() { err := receiver.handleMessage(message) if err != nil { - slog.Error("Unable to handle message", slog.Any("error", err)) + slog.Error("Unable to handle message", slog.Any(utils.ErrorKey, err)) } }() } diff --git a/src/secrets/azure_secret_getter.go b/src/secrets/azure_secret_getter.go index 06c7e8e2..de0f2ed1 100644 --- a/src/secrets/azure_secret_getter.go +++ b/src/secrets/azure_secret_getter.go @@ -6,6 +6,7 @@ import ( "github.com/Azure/azure-sdk-for-go/sdk/azcore/policy" "github.com/Azure/azure-sdk-for-go/sdk/azidentity" "github.com/Azure/azure-sdk-for-go/sdk/security/keyvault/azsecrets" + "github.com/CDCgov/reportstream-sftp-ingestion/utils" "github.com/golang-jwt/jwt/v5" "log/slog" "os" @@ -28,7 +29,7 @@ func NewSecretGetter() (SecretGetter, error) { }) if err != nil { - slog.Error("failed to obtain a credential: ", slog.Any("error", err)) + slog.Error("failed to obtain a credential: ", slog.Any(utils.ErrorKey, err)) return SecretGetter{}, err } @@ -37,7 +38,7 @@ func NewSecretGetter() (SecretGetter, error) { // Establish a connection to the Key Vault client newClient, err := azsecrets.NewClient(vaultURI, cred, nil) if err != nil { - slog.Error("failed to create a client: ", slog.Any("error", err)) + slog.Error("failed to create a client: ", slog.Any(utils.ErrorKey, err)) return SecretGetter{}, err } @@ -65,7 +66,7 @@ func (credentialGetter SecretGetter) GetPrivateKey(privateKeyName string) (*rsa. func (credentialGetter SecretGetter) GetSecret(secretName string) (string, error) { secretResponse, err := credentialGetter.client.GetSecret(context.TODO(), secretName, "", nil) if err != nil { - slog.Error("failed to get the secret ", slog.Any("error", err)) + slog.Error("failed to get the secret ", slog.Any(utils.ErrorKey, err)) return "", err } return *secretResponse.Secret.Value, err diff --git a/src/utils/credential_getter.go b/src/secrets/credential_getter.go similarity index 81% rename from src/utils/credential_getter.go rename to src/secrets/credential_getter.go index 17219652..ae218b6f 100644 --- a/src/utils/credential_getter.go +++ b/src/secrets/credential_getter.go @@ -1,8 +1,7 @@ -package utils +package secrets import ( "crypto/rsa" - "github.com/CDCgov/reportstream-sftp-ingestion/secrets" "log/slog" "os" ) @@ -24,11 +23,11 @@ func GetCredentialGetter() (CredentialGetter, error) { if environment == "local" { slog.Info("Using local credentials") - credentialGetter = secrets.CredentialGetter{} + credentialGetter = LocalCredentialGetter{} } else { slog.Info("Using Azure credentials") var err error - credentialGetter, err = secrets.NewSecretGetter() + credentialGetter, err = NewSecretGetter() if err != nil { return nil, err } diff --git a/src/utils/credential_getter_test.go b/src/secrets/credential_getter_test.go similarity index 98% rename from src/utils/credential_getter_test.go rename to src/secrets/credential_getter_test.go index f6756648..88921a49 100644 --- a/src/utils/credential_getter_test.go +++ b/src/secrets/credential_getter_test.go @@ -1,4 +1,4 @@ -package utils +package secrets import ( "bytes" diff --git a/src/secrets/local_credential_getter.go b/src/secrets/local_credential_getter.go index 069f7525..7c80f2e8 100644 --- a/src/secrets/local_credential_getter.go +++ b/src/secrets/local_credential_getter.go @@ -8,10 +8,10 @@ import ( "path/filepath" ) -type CredentialGetter struct { +type LocalCredentialGetter struct { } -func (credentialGetter CredentialGetter) GetPrivateKey(privateKeyName string) (*rsa.PrivateKey, error) { +func (credentialGetter LocalCredentialGetter) GetPrivateKey(privateKeyName string) (*rsa.PrivateKey, error) { slog.Info("Reading private key from local hard drive", slog.String("name", privateKeyName)) pem, err := credentialGetter.GetSecret(privateKeyName) @@ -27,7 +27,7 @@ func (credentialGetter CredentialGetter) GetPrivateKey(privateKeyName string) (* return key, nil } -func (credentialGetter CredentialGetter) GetSecret(secretName string) (string, error) { +func (credentialGetter LocalCredentialGetter) GetSecret(secretName string) (string, error) { slog.Info("Reading secret from local hard drive", slog.String("name", secretName)) secret, err := os.ReadFile(filepath.Join("mock_credentials", secretName)) diff --git a/src/senders/report_stream_sender.go b/src/senders/report_stream_sender.go index de5a95ee..33289633 100644 --- a/src/senders/report_stream_sender.go +++ b/src/senders/report_stream_sender.go @@ -4,6 +4,7 @@ import ( "bytes" "encoding/json" "errors" + "github.com/CDCgov/reportstream-sftp-ingestion/secrets" "github.com/CDCgov/reportstream-sftp-ingestion/utils" "github.com/golang-jwt/jwt/v5" "github.com/google/uuid" @@ -20,18 +21,13 @@ type Sender struct { baseUrl string privateKeyName string clientName string - credentialGetter utils.CredentialGetter + credentialGetter secrets.CredentialGetter } func NewSender() (Sender, error) { - environment := os.Getenv("ENV") - if environment == "" { - environment = "local" - } - - credentialGetter, err := utils.GetCredentialGetter() + credentialGetter, err := secrets.GetCredentialGetter() if err != nil { - slog.Error("Unable to initialize credential getter", slog.Any("error", err)) + slog.Error("Unable to initialize credential getter", slog.Any(utils.ErrorKey, err)) return Sender{}, err } @@ -97,7 +93,7 @@ func (sender Sender) getToken() (string, error) { res, err := http.DefaultClient.Do(req) if err != nil { - slog.Error("error calling token endpoint", slog.Any("error", err)) + slog.Error("error calling token endpoint", slog.Any(utils.ErrorKey, err)) return "", err } diff --git a/src/senders/report_stream_sender_test.go b/src/senders/report_stream_sender_test.go index 6f06c5af..8d545bc5 100644 --- a/src/senders/report_stream_sender_test.go +++ b/src/senders/report_stream_sender_test.go @@ -4,6 +4,7 @@ import ( "crypto/rand" "crypto/rsa" "errors" + "github.com/CDCgov/reportstream-sftp-ingestion/utils" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" "github.com/stretchr/testify/suite" @@ -308,7 +309,7 @@ func (suite *SenderTestSuite) Test_Sender_SendMessage_returnErrorWhenUnableToGet testKey, err := rsa.GenerateKey(rand.Reader, 2048) assert.NoError(suite.T(), err) - mockCredentialGetter.On("GetPrivateKey", "key").Return(testKey, errors.New("error")) + mockCredentialGetter.On("GetPrivateKey", "key").Return(testKey, errors.New(utils.ErrorKey)) message, _ := os.ReadFile(filepath.Join("..", "..", "mock_data", "order_message.hl7")) diff --git a/src/sftp/sftp.go b/src/sftp/sftp.go index eedee3ce..fa39c117 100644 --- a/src/sftp/sftp.go +++ b/src/sftp/sftp.go @@ -1,9 +1,11 @@ package sftp import ( + "github.com/CDCgov/reportstream-sftp-ingestion/secrets" "github.com/CDCgov/reportstream-sftp-ingestion/storage" "github.com/CDCgov/reportstream-sftp-ingestion/usecases" "github.com/CDCgov/reportstream-sftp-ingestion/utils" + "github.com/CDCgov/reportstream-sftp-ingestion/zip" "github.com/pkg/sftp" "golang.org/x/crypto/ssh" "io" @@ -28,9 +30,9 @@ type SftpClient interface { func NewSftpHandler() (*SftpHandler, error) { // TODO - pass in info about what customer we're using (and thus what URL/key/password to use) - credentialGetter, err := utils.GetCredentialGetter() + credentialGetter, err := secrets.GetCredentialGetter() if err != nil { - slog.Error("Unable to initialize credential getter", slog.Any("error", err)) + slog.Error("Unable to initialize credential getter", slog.Any(utils.ErrorKey, err)) return nil, err } @@ -44,7 +46,7 @@ func NewSftpHandler() (*SftpHandler, error) { serverKey, err := credentialGetter.GetSecret(serverKeyName) if err != nil { - slog.Error("Unable to get SFTP_SERVER_PUBLIC_KEY_NAME", slog.String("KeyName", serverKeyName), slog.Any("error", err)) + slog.Error("Unable to get SFTP_SERVER_PUBLIC_KEY_NAME", slog.String("KeyName", serverKeyName), slog.Any(utils.ErrorKey, err)) return nil, err } @@ -65,19 +67,19 @@ func NewSftpHandler() (*SftpHandler, error) { sshClient, err := ssh.Dial("tcp", os.Getenv("SFTP_SERVER_ADDRESS"), config) if err != nil { - slog.Error("Failed to make SSH client", slog.Any("error", err)) + slog.Error("Failed to make SSH client", slog.Any(utils.ErrorKey, err)) return nil, err } sftpClient, err := sftp.NewClient(sshClient) if err != nil { - slog.Error("Failed to make SFTP client ", slog.Any("error", err)) + slog.Error("Failed to make SFTP client ", slog.Any(utils.ErrorKey, err)) return nil, err } blobHandler, err := storage.NewAzureBlobHandler() if err != nil { - slog.Error("Failed to init Azure blob client", slog.Any("error", err)) + slog.Error("Failed to init Azure blob client", slog.Any(utils.ErrorKey, err)) return nil, err } @@ -94,25 +96,25 @@ func NewSftpHandler() (*SftpHandler, error) { func getSshClientHostKeyCallback(serverKey string) (ssh.HostKeyCallback, error) { pk, _, _, _, err := ssh.ParseAuthorizedKey([]byte(serverKey)) if err != nil { - slog.Error("Failed to parse authorized key", slog.Any("error", err)) + slog.Error("Failed to parse authorized key", slog.Any(utils.ErrorKey, err)) return nil, err } return ssh.FixedHostKey(pk), nil } -func getPublicKeysForSshClient(credentialGetter utils.CredentialGetter) (ssh.Signer, error) { +func getPublicKeysForSshClient(credentialGetter secrets.CredentialGetter) (ssh.Signer, error) { secretName := os.Getenv("SFTP_KEY_NAME") key, err := credentialGetter.GetSecret(secretName) if err != nil { - slog.Error("Unable to retrieve SFTP Key", slog.String("KeyName", secretName), slog.Any("error", err)) + slog.Error("Unable to retrieve SFTP Key", slog.String("KeyName", secretName), slog.Any(utils.ErrorKey, err)) return nil, err } pem, err := ssh.ParsePrivateKey([]byte(key)) if err != nil { - slog.Error("Unable to parse private key", slog.Any("error", err)) + slog.Error("Unable to parse private key", slog.Any(utils.ErrorKey, err)) return nil, err } return pem, err @@ -121,11 +123,11 @@ func getPublicKeysForSshClient(credentialGetter utils.CredentialGetter) (ssh.Sig func (receiver *SftpHandler) Close() { err := receiver.sftpClient.Close() if err != nil { - slog.Error("Failed to close SFTP client", slog.Any("error", err)) + slog.Error("Failed to close SFTP client", slog.Any(utils.ErrorKey, err)) } err = receiver.sshClient.Close() if err != nil { - slog.Error("Failed to close SSH client", slog.Any("error", err)) + slog.Error("Failed to close SSH client", slog.Any(utils.ErrorKey, err)) } slog.Info("SFTP handler closed") } @@ -137,7 +139,7 @@ func (receiver *SftpHandler) CopyFiles() { //readDir using sftp client fileInfos, err := receiver.sftpClient.ReadDir(directory) if err != nil { - slog.Error("Failed to read directory ", slog.Any("error", err)) + slog.Error("Failed to read directory ", slog.Any(utils.ErrorKey, err)) return } @@ -168,21 +170,21 @@ func (receiver *SftpHandler) copySingleFile(fileInfo os.FileInfo, index int, dir file, err := receiver.sftpClient.Open(directory + "/" + fileInfo.Name()) if err != nil { - slog.Error("Failed to open file", slog.Any("error", err)) + slog.Error("Failed to open file", slog.Any(utils.ErrorKey, err)) return } slog.Info("file opened", slog.String("name", fileInfo.Name()), slog.Any("file", file)) fileBytes, err := receiver.ioClient.ReadBytesFromFile(file) if err != nil { - slog.Error("Failed to read file", slog.Any("error", err)) + slog.Error("Failed to read file", slog.Any(utils.ErrorKey, err)) return } // TODO - build a better path (unzip? import? how do we know?) err = receiver.blobHandler.UploadFile(fileBytes, fileInfo.Name()) if err != nil { - slog.Error("Failed to upload file", slog.Any("error", err)) + slog.Error("Failed to upload file", slog.Any(utils.ErrorKey, err)) } slog.Info("About to consider whether this is a zip", slog.String("file name", fileInfo.Name())) @@ -190,16 +192,23 @@ func (receiver *SftpHandler) copySingleFile(fileInfo os.FileInfo, index int, dir // write file to local filesystem err = os.WriteFile(fileInfo.Name(), fileBytes, 0644) // permissions = owner read/write, group read, other read if err != nil { - slog.Error("Failed to write file", slog.Any("error", err), slog.String("name", fileInfo.Name())) + slog.Error("Failed to write file", slog.Any(utils.ErrorKey, err), slog.String("name", fileInfo.Name())) return } - _ = usecases.UnzipProtected(fileInfo.Name()) + zipHandler, err := zip.NewZipHandler() + + if err != nil { + slog.Error("Failed to init zip handler", slog.Any(utils.ErrorKey, err)) + return + } + + _ = zipHandler.Unzip(fileInfo.Name()) //delete file from local filesystem err = os.Remove(fileInfo.Name()) if err != nil { - slog.Error("Failed to remove file", slog.Any("error", err), slog.String("name", fileInfo.Name())) + slog.Error("Failed to remove file", slog.Any(utils.ErrorKey, err), slog.String("name", fileInfo.Name())) } } diff --git a/src/sftp/sftp_test.go b/src/sftp/sftp_test.go index 9f07c454..213d284f 100644 --- a/src/sftp/sftp_test.go +++ b/src/sftp/sftp_test.go @@ -5,6 +5,7 @@ import ( "crypto/rsa" "errors" "github.com/CDCgov/reportstream-sftp-ingestion/mocks" + "github.com/CDCgov/reportstream-sftp-ingestion/utils" "github.com/pkg/sftp" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" @@ -51,7 +52,7 @@ func Test_getPublicKeysForSshClient_returnErrorWhenUnableToRetrieveSFTPKey(t *te defer os.Unsetenv("SFTP_KEY_NAME") mockCredentialGetter := new(MockCredentialGetter) - mockCredentialGetter.On("GetSecret", mock.Anything).Return("", errors.New("error")) + mockCredentialGetter.On("GetSecret", mock.Anything).Return("", errors.New(utils.ErrorKey)) pem, err := getPublicKeysForSshClient(mockCredentialGetter) @@ -128,7 +129,7 @@ func Test_CopyFiles_failToReadDirectory(t *testing.T) { files = append(files, fileInfo) - mockSftpClient.On("ReadDir", mock.Anything).Return(files, errors.New("error")) + mockSftpClient.On("ReadDir", mock.Anything).Return(files, errors.New(utils.ErrorKey)) sftpHandler := SftpHandler{sftpClient: mockSftpClient} @@ -207,7 +208,7 @@ func Test_copySingleFile_failedToOpenFile(t *testing.T) { slog.SetDefault(slog.New(slog.NewTextHandler(buffer, nil))) mockSftpClient := new(MockSftpClient) - mockSftpClient.On("Open", mock.Anything).Return(&sftp.File{}, errors.New("error")) + mockSftpClient.On("Open", mock.Anything).Return(&sftp.File{}, errors.New(utils.ErrorKey)) fileDirectory := filepath.Join("..", "..", "mock_data") filePath := filepath.Join(fileDirectory, "copy_file_test.txt") @@ -236,7 +237,7 @@ func Test_copySingleFile_failedToReadFile(t *testing.T) { mockIoWrapper := new(MockIoWrapper) var emptyBytes []byte - mockIoWrapper.On("ReadBytesFromFile", mock.Anything).Return(emptyBytes, errors.New("error")) + mockIoWrapper.On("ReadBytesFromFile", mock.Anything).Return(emptyBytes, errors.New(utils.ErrorKey)) fileDirectory := filepath.Join("..", "..", "mock_data") filePath := filepath.Join(fileDirectory, "copy_file_test.txt") @@ -273,7 +274,7 @@ func Test_copySingleFile_failToUploadFile(t *testing.T) { mockIoWrapper.On("ReadBytesFromFile", mock.Anything).Return(fileBytes, nil) mockBlobHandler := &mocks.MockBlobHandler{} - mockBlobHandler.On("UploadFile", mock.Anything, mock.Anything).Return(errors.New("error")) + mockBlobHandler.On("UploadFile", mock.Anything, mock.Anything).Return(errors.New(utils.ErrorKey)) sftpHandler := SftpHandler{sftpClient: mockSftpClient, blobHandler: mockBlobHandler, ioClient: mockIoWrapper} sftpHandler.copySingleFile(fileInfo, 1, fileDirectory) diff --git a/src/storage/azure.go b/src/storage/azure.go index d288c1e5..3f1945c9 100644 --- a/src/storage/azure.go +++ b/src/storage/azure.go @@ -26,7 +26,7 @@ func NewAzureBlobHandler() (AzureBlobHandler, error) { func (receiver AzureBlobHandler) FetchFile(sourceUrl string) ([]byte, error) { sourceUrlParts, err := azblob.ParseURL(sourceUrl) if err != nil { - slog.Error("Unable to parse source URL", slog.String("sourceUrl", sourceUrl), slog.Any("error", err)) + slog.Error("Unable to parse source URL", slog.String("sourceUrl", sourceUrl), slog.Any(utils.ErrorKey, err)) return nil, err } @@ -46,7 +46,7 @@ func (receiver AzureBlobHandler) FetchFile(sourceUrl string) ([]byte, error) { func (receiver AzureBlobHandler) UploadFile(fileBytes []byte, blobPath string) error { uploadResponse, err := receiver.blobClient.UploadBuffer(context.Background(), utils.ContainerName, blobPath, fileBytes, nil) if err != nil { - slog.Error("Unable to upload file", slog.String("destinationUrl", blobPath), slog.Any("error", err)) + slog.Error("Unable to upload file", slog.String("destinationUrl", blobPath), slog.Any(utils.ErrorKey, err)) return err } @@ -58,19 +58,19 @@ func (receiver AzureBlobHandler) UploadFile(fileBytes []byte, blobPath string) e func (receiver AzureBlobHandler) MoveFile(sourceUrl string, destinationUrl string) error { sourceUrlParts, err := azblob.ParseURL(sourceUrl) if err != nil { - slog.Error("Unable to parse source URL", slog.String("sourceUrl", sourceUrl), slog.Any("error", err)) + slog.Error("Unable to parse source URL", slog.String("sourceUrl", sourceUrl), slog.Any(utils.ErrorKey, err)) return err } destinationUrlParts, err := azblob.ParseURL(destinationUrl) if err != nil { - slog.Error("Unable to parse destination URL", slog.String("destinationUrl", destinationUrl), slog.Any("error", err)) + slog.Error("Unable to parse destination URL", slog.String("destinationUrl", destinationUrl), slog.Any(utils.ErrorKey, err)) return err } fileBytes, err := receiver.FetchFile(sourceUrl) if err != nil { - slog.Error("Unable to fetch file", slog.String("sourceUrl", sourceUrl), slog.Any("error", err)) + slog.Error("Unable to fetch file", slog.String("sourceUrl", sourceUrl), slog.Any(utils.ErrorKey, err)) return err } @@ -81,7 +81,7 @@ func (receiver AzureBlobHandler) MoveFile(sourceUrl string, destinationUrl strin _, err = receiver.blobClient.DeleteBlob(context.Background(), sourceUrlParts.ContainerName, sourceUrlParts.BlobName, nil) if err != nil { - slog.Error("Error deleting source file after copy", slog.String("source URL", sourceUrl), slog.Any("error", err)) + slog.Error("Error deleting source file after copy", slog.String("source URL", sourceUrl), slog.Any(utils.ErrorKey, err)) return err } diff --git a/src/usecases/read_and_send.go b/src/usecases/read_and_send.go index 6426be10..bb76886d 100644 --- a/src/usecases/read_and_send.go +++ b/src/usecases/read_and_send.go @@ -3,8 +3,7 @@ package usecases import ( "github.com/CDCgov/reportstream-sftp-ingestion/senders" "github.com/CDCgov/reportstream-sftp-ingestion/storage" - "github.com/yeka/zip" - "io" + "github.com/CDCgov/reportstream-sftp-ingestion/utils" "log/slog" "os" "strings" @@ -22,7 +21,7 @@ type ReadAndSendUsecase struct { func NewReadAndSendUsecase() (ReadAndSendUsecase, error) { blobHandler, err := storage.NewAzureBlobHandler() if err != nil { - slog.Error("Failed to init Azure blob client", slog.Any("error", err)) + slog.Error("Failed to init Azure blob client", slog.Any(utils.ErrorKey, err)) return ReadAndSendUsecase{}, err } @@ -36,7 +35,7 @@ func NewReadAndSendUsecase() (ReadAndSendUsecase, error) { slog.Info("Found REPORT_STREAM_URL_PREFIX, will send to ReportStream") messageSender, err = senders.NewSender() if err != nil { - slog.Warn("Failed to construct the ReportStream senders", slog.Any("error", err)) + slog.Warn("Failed to construct the ReportStream senders", slog.Any(utils.ErrorKey, err)) return ReadAndSendUsecase{}, err } } @@ -54,19 +53,19 @@ func NewReadAndSendUsecase() (ReadAndSendUsecase, error) { func (receiver *ReadAndSendUsecase) ReadAndSend(sourceUrl string) error { content, err := receiver.blobHandler.FetchFile(sourceUrl) if err != nil { - slog.Error("Failed to read the file", slog.String("filepath", sourceUrl), slog.Any("error", err)) + slog.Error("Failed to read the file", slog.String("filepath", sourceUrl), slog.Any(utils.ErrorKey, err)) return err } reportId, err := receiver.messageSender.SendMessage(content) if err != nil { - slog.Error("Failed to send the file to ReportStream", slog.Any("error", err), slog.String("sourceUrl", sourceUrl)) + slog.Error("Failed to send the file to ReportStream", slog.Any(utils.ErrorKey, err), slog.String("sourceUrl", sourceUrl)) // As of June 2024, only the 400 response triggers a move to the `failure` folder. Returning `nil` will let // queue.go delete the queue message so that it will stop retrying // We're treating all other errors as unexpected (and possibly transient) for now - if strings.Contains(err.Error(), "400") { - receiver.moveFile(sourceUrl, "failure") + if strings.Contains(err.Error(), utils.ReportStreamNonTransientFailure) { + receiver.moveFile(sourceUrl, utils.FailureFolder) return nil } @@ -76,13 +75,13 @@ func (receiver *ReadAndSendUsecase) ReadAndSend(sourceUrl string) error { slog.Info("File sent to ReportStream", slog.String("reportId", reportId)) - receiver.moveFile(sourceUrl, "success") + receiver.moveFile(sourceUrl, utils.SuccessFolder) return nil } func (receiver *ReadAndSendUsecase) moveFile(sourceUrl string, newFolderName string) { - destinationUrl := strings.Replace(sourceUrl, "import", newFolderName, 1) + destinationUrl := strings.Replace(sourceUrl, utils.MessageStartingFolderPath, newFolderName, 1) if destinationUrl == sourceUrl { slog.Error("Unexpected source URL, did not move", slog.String("sourceUrl", sourceUrl)) @@ -92,42 +91,6 @@ func (receiver *ReadAndSendUsecase) moveFile(sourceUrl string, newFolderName str // After successful message handling, move source file err := receiver.blobHandler.MoveFile(sourceUrl, destinationUrl) if err != nil { - slog.Error("Failed to move file after processing", slog.Any("error", err)) + slog.Error("Failed to move file after processing", slog.Any(utils.ErrorKey, err)) } } - -// TODO - this probably belongs in a different file -func UnzipProtected(zipFilePath string) error { - slog.Info("Called unzip protected") - zipReader, err := zip.OpenReader(zipFilePath) - - if err != nil { - slog.Error("Failed to open zip reader", slog.Any("error", err)) - return err - } - defer zipReader.Close() - - for _, f := range zipReader.File { - slog.Info("inside of zip Reader loop") - // TODO - should we warn or error if not encrypted? This would vary per customer - if f.IsEncrypted() { - f.SetPassword("test123") - } - slog.Info("setting password") - fileReader, err := f.Open() - if err != nil { - slog.Error("Failed to open file", slog.Any("error", err)) - } - defer fileReader.Close() - - slog.Info("file opened", slog.Any("file", f)) - buf, err := io.ReadAll(fileReader) - - slog.Info(string(buf)) - - if err != nil { - slog.Error("Failed to read file", slog.Any("error", err)) - } - } - return nil -} diff --git a/src/utils/constants.go b/src/utils/constants.go index 6f2ecfac..c5171e61 100644 --- a/src/utils/constants.go +++ b/src/utils/constants.go @@ -1,3 +1,23 @@ package utils +// The name of the Azure blob storage container. In future, there will be different ones per customer const ContainerName = "sftp" + +// HL7 messages (NO zips!) placed in this folder trigger a queue message. +// We read the message and send it to ReportStream +const MessageStartingFolderPath = "import" + +// HL7 messages are moved from the `MessageStartingFolderPath` to the `SuccessFolder` after +// we receive a success response from ReportStream +const SuccessFolder = "success" + +// HL7 messages are moved from the `MessageStartingFolderPath` to the `FailureFolder` after +// we receive a failure response from ReportStream +const FailureFolder = "failure" + +// In read_and_send, move files to the `FailureFolder` when we get the below response from ReportStream +const ReportStreamNonTransientFailure = "400" + +// Use this when logging an error. +// E.g. `slog.Warn("Failed to construct the ReportStream senders", slog.Any(utils.ErrorKey, err))` +const ErrorKey = "error" diff --git a/src/zip/zip.go b/src/zip/zip.go new file mode 100644 index 00000000..1684daa3 --- /dev/null +++ b/src/zip/zip.go @@ -0,0 +1,102 @@ +package zip + +import ( + "github.com/CDCgov/reportstream-sftp-ingestion/secrets" + "github.com/CDCgov/reportstream-sftp-ingestion/storage" + "github.com/CDCgov/reportstream-sftp-ingestion/usecases" + "github.com/CDCgov/reportstream-sftp-ingestion/utils" + "github.com/yeka/zip" + "io" + "log/slog" + "os" +) + +type ZipHandler struct { + credentialGetter secrets.CredentialGetter + blobHandler usecases.BlobHandler +} + +type ZipClient interface { +} + +func NewZipHandler() (ZipHandler, error) { + // environment := os.Getenv("ENV") + + // TODO - Address local implementation vs test behavior + + //if environment == "local" { + // blobHandler, err := storage.NewAzureBlobHandler() + //} + + blobHandler, err := storage.NewAzureBlobHandler() + if err != nil { + slog.Error("Failed to init Azure blob client", slog.Any(utils.ErrorKey, err)) + return ZipHandler{}, err + } + + credentialGetter, err := secrets.GetCredentialGetter() + if err != nil { + slog.Error("Unable to initialize credential getter", slog.Any(utils.ErrorKey, err)) + return ZipHandler{}, err + } + + return ZipHandler{ + credentialGetter: credentialGetter, + blobHandler: blobHandler, + }, nil +} + +// TODO - refactor for tests? +// TODO - tests +// TODO - move remaining items to future cards? +// TODO - check storage size/costs on the container +// TODO - update CA password after deploy per env +func (zipHandler ZipHandler) Unzip(zipFilePath string) error { + secretName := os.Getenv("CA_DPH_ZIP_PASSWORD_NAME") + zipPassword, err := zipHandler.credentialGetter.GetSecret(secretName) + + if err != nil { + slog.Error("Unable to get zip password", slog.Any(utils.ErrorKey, err)) + return err + } + + slog.Info("Called unzip protected") + zipReader, err := zip.OpenReader(zipFilePath) + + if err != nil { + slog.Error("Failed to open zip reader", slog.Any(utils.ErrorKey, err)) + return err + } + defer zipReader.Close() + + for _, f := range zipReader.File { + slog.Info("inside of zip Reader loop") + // TODO - should we warn or error if not encrypted? This would vary per customer + if f.IsEncrypted() { + f.SetPassword(zipPassword) + } + slog.Info("setting password") + fileReader, err := f.Open() + if err != nil { + slog.Error("Failed to open file", slog.Any(utils.ErrorKey, err)) + } + defer fileReader.Close() + + slog.Info("file opened", slog.Any("file", f)) + buf, err := io.ReadAll(fileReader) + + slog.Info(string(buf)) + + if err != nil { + slog.Error("Failed to read file", slog.Any(utils.ErrorKey, err)) + } + + err = zipHandler.blobHandler.UploadFile(buf, utils.MessageStartingFolderPath+"/"+f.FileInfo().Name()) + + if err != nil { + slog.Error("Failed to upload file", slog.Any(utils.ErrorKey, err)) + } + + } + return nil +} From 50ca64a21c308fbae4619738e2e88f724f5a8b7a Mon Sep 17 00:00:00 2001 From: Sylvie Date: Tue, 2 Jul 2024 16:24:16 -0500 Subject: [PATCH 03/12] Start adding tests to zip.go, refactor out mock credential getter so it can be reused Co-Authored-By: pluckyswan <96704946+pluckyswan@users.noreply.github.com> Co-Authored-By: Samuel Aquino Co-Authored-By: jcrichlake <145698165+jcrichlake@users.noreply.github.com> Co-Authored-By: jherrflexion <118225331+jherrflexion@users.noreply.github.com> --- src/mocks/mock_credential_getter.go | 19 +++++ src/mocks/test_data/passworded.zip | Bin 0 -> 1048 bytes src/mocks/test_data/unprotected.zip | Bin 0 -> 2044 bytes src/senders/report_stream_sender_test.go | 39 ++++------ src/zip/zip.go | 38 +++++----- src/zip/zip_test.go | 92 +++++++++++++++++++++++ 6 files changed, 144 insertions(+), 44 deletions(-) create mode 100644 src/mocks/mock_credential_getter.go create mode 100644 src/mocks/test_data/passworded.zip create mode 100644 src/mocks/test_data/unprotected.zip create mode 100644 src/zip/zip_test.go diff --git a/src/mocks/mock_credential_getter.go b/src/mocks/mock_credential_getter.go new file mode 100644 index 00000000..a582e2f9 --- /dev/null +++ b/src/mocks/mock_credential_getter.go @@ -0,0 +1,19 @@ +package mocks + +import ( + "crypto/rsa" + "github.com/stretchr/testify/mock" +) + +type MockCredentialGetter struct { + mock.Mock +} + +func (m *MockCredentialGetter) GetPrivateKey(privateKeyName string) (*rsa.PrivateKey, error) { + args := m.Called(privateKeyName) + return args.Get(0).(*rsa.PrivateKey), args.Error(1) +} +func (m *MockCredentialGetter) GetSecret(secretName string) (string, error) { + args := m.Called(secretName) + return args.Get(0).(string), args.Error(1) +} diff --git a/src/mocks/test_data/passworded.zip b/src/mocks/test_data/passworded.zip new file mode 100644 index 0000000000000000000000000000000000000000..ff239aca30c6e8e127a0bbf8a64c2ec9a3fabb54 GIT binary patch literal 1048 zcmWIWW@h1H;ACK6;7Yp`VR}M;ksy%G3dAA|G7P!J=}CzxMg~SEx(4RDMh1ErIp(1u zoD9s#CkoR zbs|9PI2gW7JQ6W+;=Z%RObiT3EDQ|N5bM(O^O5WWT3%X|1GiOvL3~uVVt zr+zux&nd&V*Pn@Po59W4X`3BC=4x)WubP!V!*dhg`wMcNldWCQ${VR7z$SA@<@#K1wzCR) z8$Ubfn14T6zp?AtU;euv6JH1B-G4jvAZyH=w3R+n6yErLIDY?JLB(qOZ<$}7a@gyv z^1`;VOI4~ z^Tg4n`B%y=Jbx(k!-LZ{%RgD>S-Veou$*W=n~sM=_4$WiHnmQ@mEyCmZ{4)xhNa8j zI6nJf)9bu_-J?&!EWzq;rqzjBi!@6AmXnLQS9I60L-W{Eg%4)lPP_LM8vomBGWW-i zWPO3~*!8h{OQ*7EfB0!#zi-LaFNangZ^`|z%J}0n&%l()r4?nOZ}%mM#zZa<-6_sm zm+^A`)hh?}w5{KWz2BZ{w3Lf^dG7i=lWlLTHruVJ7Mx=}S!wmWFDiz~Z+2{RGhcA) z|M2enb6^=bOWinmzP}pl2f+D+|~c?G23na?_bmR&V5pBVsh+Fs>SqU zM=h&DUN5Zu^?8<$!bZKtA3FO_G9|m@EeyWZvKDjq+O^4AV&itmj!|zz~L%uc?`(IXh_pLZ* z`NH;=*KHGwxAGkExR>cU@21cJsn5cDmrqUF&t4xo1C*|jQ*VGbBa<96u8brB%o+>~ zz)Zrhq!Glzo~>9R*$OR7K}^P#XOK$rTQ-)hxZkC?Ou5PjrQJR{$|K;+H zt5$Dp{I&JS*9!;DLanAg^>d8(U9Mc0vFh-bhW=nv(;crrzTDw3N#yDBzYEs}?qCRr zdE1znHUH;xK8fTTv%H;RHf=CEWFdG&t*P=#-0Vpib458*TAEIlt7ZGeJ~BFgac5dY z!m6#)KK?l)c=~K?+?#iQbWUzI@$lZE@^szljqR~!pKZ^+F#0V$L-#?bT<%uI#Z!cy zS;#T`aGfWjsnU8)tMinAne>trMdumHb@xJwq*)il9f_PWTmHl59Y?n4v^cDqlGQwnz0xVhZH@s7`#GA*1-u2eC7ippxs};k^`JmXY9_LzvOzQ zUh>$wS+^Ft|GQASeBa!C3$rEN{_!4p+1lEo^Dg#B*7@1#@w%^h3fGynC;sT^>pz@g z=XXnUWO$FqNTlcm9 z-uNov`TI$&Houpyp5FY|^4{v8(>f8g?4a~LapJzS=}Zg^pj3=KeTx7~6f=T3m#Cyp zS_W2DVEZKLD$&8zr>N*CAQo|4PNB~s_z;VzctXMiuXE?Gdh2MM@Vt83Q%6rz*U#6} zQ^zxukE3msE%OmM9-coOr8_%MS~@uH7jDf|U{!42bCBz%gxO!OO`iK-e@&cSz2wun zlIF`fJndq3r{|n`T2xw?v;5??a`|lnkL$o$pYdfu?Q~$a2L(WYHzSh>Gw#wEs0<9= zI)W(lvW;+A4KW>7S|b;ypwb!!mNfcdF`Zz!392E`%1xMmU^XHbbD&}x29`7)2QsN# zXd_z=DoK&kAgCmTfhCO#88HJB=v|@`8 Date: Tue, 2 Jul 2024 17:20:46 -0500 Subject: [PATCH 04/12] Make test naming consistent Co-Authored-By: pluckyswan <96704946+pluckyswan@users.noreply.github.com> --- adr/008-test-naming-convention.md | 15 +++++++ src/mocks/test_data/non-zip-file.zip | Bin 0 -> 11595 bytes src/orchestration/queue_test.go | 48 +++++++++++------------ src/secrets/credential_getter_test.go | 6 +-- src/senders/report_stream_sender_test.go | 28 ++++++------- src/sftp/sftp_test.go | 24 ++++++------ src/usecases/read_and_send_test.go | 16 ++++---- src/zip/zip.go | 1 + src/zip/zip_test.go | 4 +- 9 files changed, 79 insertions(+), 63 deletions(-) create mode 100644 adr/008-test-naming-convention.md create mode 100644 src/mocks/test_data/non-zip-file.zip diff --git a/adr/008-test-naming-convention.md b/adr/008-test-naming-convention.md new file mode 100644 index 00000000..73d27bc4 --- /dev/null +++ b/adr/008-test-naming-convention.md @@ -0,0 +1,15 @@ +# 8. Test Naming Convention + +Date: 2024-07-02 + +## Decision + +For consistency, we'll name tests like +`Test_TheMethodBeingTested_TheCoveredScenario_TheExpectedResult`, e.g. +`Test_Unzip_FileIsPasswordProtected_UnzipsSuccessfully`. For a baseline scenario test, +it's okay to do something like `Test_TheMethodBeingTested_TheExpectedResult` +e.g. `Test_getToken_ReturnsAccessToken`. + +## Status + +Accepted. diff --git a/src/mocks/test_data/non-zip-file.zip b/src/mocks/test_data/non-zip-file.zip new file mode 100644 index 0000000000000000000000000000000000000000..e3fa76a8b8240577e4847b75c0d9b6ff8d1b2f50 GIT binary patch literal 11595 zcmeI2SyYo(|L>n9Pm(7SBq2eD2u}te!H6Lsf?`cVfDj`lh!hmtfT%dN1Z_p@@MJ(j zKn#NlmTEv$)Y<}4OC94?%lg56biGlvKB5}n3R;n=ko(*&V2Ul z+2hBL2L=XweSJUp;Deuj`YC*t&#G0cLPJ9}8qJyvF_X#sXkF~>+qWM)c+k_+bN%}D z^XJb$efpF}qXh>C@7N?wOG|5QZH-^Zt*x!~pm?MzLpwS;8XFs(PG?_VpUdUix^-(y zOUv1_XZP;iTVG!f!|?9iyLCF9*=!~d2wh!WcDw!b>C*)T1q-8Ki^XCxne=+SLdx&$ z?d5PdHk*yd^%t?i31z5oE=({Seym;}~Uw_S+Gw0V|e|_}m(Qm)~Haa@`#~*);jg3uAObiYVK79Ca z_Uzd~K|ymP=Z1uY_{{KO`LoC*vRp1-mMk3~f3bAwQVyFlHabS7Qezjyu3WjYv$Ip1 zx$MEi#~*(9;WyuW^YhO?mzG-cb(_|$Ti4Lg@UMUUtERf9q;&tAsi}&}s>3yh_wC!a zVZ(+OzyEHvmR-Df@%i)TU$hbQObJ5RHd{RggV|oN^{>e{zy3OQ^XAQ~ zSFc9Sjq>#LEHalgH@AHB(MMl?`Q`G|)S{xI(b39S7@fIsjj_<%2o1wLdxv&D&DHYQ26!wq>_;W zBGX<&V`44{vwqMYYo{zBKl=;lbfTGws+tNz4;G~g^UYo%3p1XgEePen%*Ja+7wDt0 zYtmJB%qkF0oz#Y^ZA6N5bM{CThu$WoTWU9z8ye0o#a8rh!{F@!W+vl<(Q-_?=P(+>Ezt64b1xP-Kj6PoHY$O_j zh2?q!Wb{$j1J_ShrpE`D1rwX-k6bXp^k+_mI;#xl6_6amedv_cTVA`qQ*bH*5+TyT z7gHPIto_@6H&NLf5PKLB`gn9UH|%2^5tZ#-CF=^zAZdykB-dsyN_U|7`0WD-z9DBI zA^%Z4Uk}mweMzjNe2Eg!$tx@{Oq2m!`jFm&Pa2HC;*P+rP!ypy8!gxj)9bb$o76Z! zrpBPhbvNYUVYNcqL)wz64q!9M>T+}>-*g2h=@^l1{N=MED1hmrUcnqu>AJlrx$2%B zCMqYvl(Lo9EhOw5pss$GJmQ#1r5lQ(-o2%kMHEH1_X;)-`LNl?%rHXzbZgZ$N?dit zl>jE68VBL4Y#rX`sUo6j9%mpS7-57L2D5ba`vk6i$5j50E=>)W!3#I#NwM?3k1ir* z7670+j7IAqo$n}zuI ztFFK1S!Yg2V@2C!J!m&NtsJv?=L$XUT+Ux%l)D$!e|dQj6f6BtnpXQ}V7}tYsoVM% zh_sh8ze}L?Ph4$6`xYz+*UqXgaS1HBu}mydErS*YzFQ@m&mkn?W zr${)mqI`V5Dc4F2rK=7n9X3^ETkwl+prEk1-D8;HhhJqWDd|JTo}cKYHk2^*H;n}! z8L{aK=yxmkryWQ-+c`6x(YzhIn!J?gId@CGgJk9Z3ztS$Dj*KdCm9xwC=rz#@> zs;x(k_3IR=a__)Af)B=8TfJDzVGE_(*0NXt=Z4O>-aL`O2 ztq)dWj}TeQ(zSNt;upq1s+y26w0gxJg#i_sWyS*f46h_m+*t0p4wlf$C%C-5S!S7c z#Z0RL?OBZgAe=?&zGVt&yJd}z=Fn=X*f6=qBkUW@la8C?>#qy_AuoB%>stQV=b(!xDYj&B%fi8yvLU|7!Ao&YfCNT8OW zL4qXp(T7v(<}Y8P@mTEZ`HFu(Yp|B+cI=;m3m?hI6b`EsHG0w@RFyk-_25pwY?ntj zRQJ3HQGYrGZ1|%H^hu>AYpy~SdZK?$-cs6$4r>R6$~dlOxvGp9o&kJZSA-~rONn2Z z;}DRQj#%KD90dPCL919&2z55ckq-~$IbZ-ca<9mcYj4Ly^;H;wWn}5~z!g}ItI{MH zM(4!vSN|-j;Ei=q2pt?LyQY-mNHrBi%h<_{I@!!OJdG(W@I2cUSSZosJ)wrY2b^25 z9SV58M%0bFgvS=9bKQ!%$> z6O8$c+0g_AL-r}#;xJ(c+p;9!`l}+?NUc|B&X#Uj6L+X98#o<=$%Qi5MQBzQYEv>pv%;ka>v%EM`S0s>B zEe<33%Fmz(`^@7>q&9cU6`pUF6C;?@=l*bAmhG>^&=u)_dh#cG%cW<%x~1);uxnTQ7BxR$#|WUHR^QoIu<< zY3w{nwuJ}H$Qd^TvJV;c;O5C;sZZp)Rsy@(M<)GdrYOR)$6(EOgqS+nCX!*(+(h9r z(^NBpBJi!uT0^ba!Pdr<7S=LzMNXKTGwLL5xiotndx>|Q=f|P) zz{UtV6zXSs4<+0e%d`th0T++)vSCCosKiUla0mB3*E5%PB1BYxSWFCHGBong(Q0SI z^I)IxK}=@!^Ey)WX#OQI9Spx*yJb*xeOYK*Q~mRR))oj-Tk{jKCsn_3V=2)GDtMqx z{4Ly=Yv=Krfdo*3IsS`=QmgYLrUD=SO)!mTHUWOIF!3T%bi6(YOK}_M?FI#hKWdt7 zY6oA!@z?}*5Q&eb@0Qp_Dg=+7%%T>!WoVql$j!lcv2!4N5z@CmOV080krdLCXdZq( zc2D(A31HEpJ+%%DM=cK`k74{ou$hzrhNM!l@v(Mbuer;UJPIt5jO-Kbl+gWb0PV%S zVvAYMOabI22!}V1iokli$08rMwdjSBQ{Pv@VhuyE)f8gqm)~yerXPwn1<%_KEgi%! zztLaK$I_|)9X2)Bcb0%NGc3mPERL#pME$fCV+dJH&YiH$JGI)mDMq$_pu%o|U>v98 zv?x)zs4R@~!G<7(p1}_t>G1vdq`FWOwn9(bq3$dp(o;XOtX}UlN9iHv?KQ=x9Z6>k zaCwYJDMroa+JIlab_yNwmq41Bne>E5XWmXtr*ld*?2o{H~m<39V1h+8czV zXue1+nefDo!HNP#S6i*^}JhM2EByc>HUWU_CVFN!syUF5PCUrtN2ElDA8Sry(P*dubeV83dr!t+RU6@`rqnn$dLYh!!;w4WPF-#Gh`npkD}Izc+LVPlT?HBXZ1yFtXZPRtai*4sM>* znDN1GeK$*A>QqJPd@#naZ*pmPjFBp9*vD<)Tn)l+=OCU(zM>t)pY*9PX5B8;Dp7UY zTNDcAE~}$*blCUjoh-%_j}U&YpaLZ5-C54C<_&AHFqg-?-J~-$Z}2x$#YMPnZ)j%F zbRR&33~H5&vRhb*4cNv*u5K@mM8)GU)CHddTXL1em>HuI1!Wsq26(Z@5JxIzSBu3O%!W z%4y!|XiQU9C2c>u~*annIH`*;(O0D^n4QHO?R;xv6 zisQJy=2^mm9(x{ZbkGa>O<^4X$ta@jgdBxFYMYK+j0wKPNA{sc?chtRbJ?$M^jnZw zY^Vh)8wx#%qDk$t`BV||o;q$WEdvA?W0*L3Hi{y_hQ`8$K%X7hJ&NH4P!UeG9g`ki zC=Ad7anvlr0=r2rLLvTul(sWc9{F02;9WGj^+h}8L?3{4jo|ZA<-FEDlp4^mmF1v# zMIHtPNpQLsMpW82%0+w*@v=)g2LynmSPpMyaa{W?4q+*4*~kga1-|{%GH7IH531@k zH?jU>`FbaLF>u^fy?!)Ln2gFyy;smf(^e5uxs1_vZvAXz5mT<6@>839jXyENA56dy z)kf5VU-^D#lSLoDs;o(Ac2t?jr&M_X@%!%|t7VwLdaXYhNAH-qvJs1eYGv@kf)RC;3CLb;k>l#W z_uqgtD??T_hKS!hdUzBL3rF`D5&zCF*v zj$xg+2AyEQha)$d8V2#c65scajQ3Az8JMHJyrHdwcyJWYdn-7qMk z(LH@sbNfOTM>>|G>N0=4&D>^9AzY{@sP#v}gjN^lkQ*?Q{8AlyZ=WILCr)vCez6^$ zk?kPOaU)GBnCkiv?{n1q1WMA@@($7KTyGlIxLhrCF8}-vE@rt8@Ae}9q$)}bQ*_R8 zMYpvN$2QvX4`m2yEA-fE(Y53=`hvca3|*5t&Y#<4Z$+Jbqq`BFnjr>+DG;ZS6B1{? z;nZ8{Bp@tnss)^JLL?5viPiZ+F-HL$Jd#o^!MJF{NIW*~SE38li297c;`F|!aG_|z zHwKeA=wuZ@UvP@zYX|m%HNE8oZq1PKq=o5bTjGms=s-&$D8`4<+6)%P9Em=W5&()J z%m1dD^1o0mB@X>ZsahVK^_FUv-cpV7zo-W61bfTvn9Y|)`9?5rC_YV6HX~0!ZEbI- z#On2ADnQMQER^alHbrI!G|qES6*I@59FXBHo`8zMyXqJ$x-q2DsV z-_QiU7g#lvMxZQ@jbk9JIQs}fAZk|wu_DL92DM%@FTCwtv2 ztZZFgeQu+MwUiRtDu_(9?`OU*_|qr#G1g$IR5YG#;wA52=`|y1WNUF8KIzA>nwizE zG=t_?eyS>x*KQ^(-=qFiv0fJB-IsigE#XKb;W}PMO6spmPwZgcgI(IwNi2^f;gZZW zL2`A}C*tJ=`o#9y87BrhpY|y+4VDsg?fj3vu0(2S%XexK_@HT_n@~LU^J}Nv3!9GS zS&XH=r+Ap@8UH$cUGu!6TNQ`Z@h zkuEWW(o`A*?uVZMgVovCMM>A7JWB~E8r#zYs_Lj-q1R{#$Q0^bOoFsQxiRi?);oxWqp;1pwT)V z=G^;+9n|t$lB?k{sxeed=oitpyaSsCu)xNY`vkfdHAP==jFIo~-%#763`w$!Puf1A z8WjFphQhH#ub>tgmab5nf%3O%Keu6O8NuTOq*VBufD}0mm6=ZuYPlro!RylFFysq9 zTR!8S%@JuWzSj26g=b9_e0dp1v`cR)pdL+~8 zAl*$NgN%{8-1aWaLY-xWIVCnHm7d#&>OpI?20=^oNIpx_ORL4*>znix2eVxE>$@dv z9OvYHZX|I_jsfoxwt$}O49jIl20iJ`QbwoxKI;xV_DpZac|zBKfGuUy8t2mvor{+r z3pfi;*4a~2EtcVq<%~RW ze|=c%NYQmVOn5t+qbJ0-0cP;%$OfWtTc2Pqma@9Swl7C(f8{ z!8DK9ql@`nb;#D!PqcADw#ngadMi?^o{Dm80l_xvl{YrZsAY%{o8}NnRu=WEwO#RjZ(Nj+}AfGzbsAgA&ChLTqKK;SeI?1qT zES6zMwcu*Rs0&L=eEm(k!!+zI-Ibtr$3UmpUlnatZN1B_x2R$S}UE3Kc6ky z25r?=CB@&2h8|Y8(AULzC1UTXrs{ckv!{lvfn-5^>+cdKv47)n{i>Vs7n46rVXV99 zp*o~)xgx}g97~1I87T%Uw2gIUsOymf^0$9Sj_^h77O&MaxW2J!8 zGe|dqI6^2vVnEQK;C~9zI;bYdYxeKmkT(x44-EjsvNf&W%ykKMlhjU*r$+KKd@c!* zVuvHK2<-d0bsvK+8aVRFbs@H@H=0!%>X+l(uyRgQT-3qJ^5r?D?15K?9Dp1iJ8lNxAzZ*#xapqBB3or3XY_MUHe#x^iTd2K*B^aqrL9B zZzu4@I=V^@5bsq|gnrUuV73jaGI%b^UGwE8g&k{{z-)e_M)7^zCNo4i8{$=%o#sxF&jH#dHBrrg~XFJRO%IavqUyM zS~aOXy~GxEg08i^{?OGF{ft*iOktxIs?lB_Z4W25pNuZf*_Rvf7+bdc>a7j0n>y0! zq71mZ((mA(aYSN5%~M%;nK@?j!#s~l*o_uU0eq!lQ3bKZcO1LcYgunRjb11{z{?tFaPCM)yQ@xnX9 zh*WV8#I)5*qZvWcj=1@gsLt)1spCbEly5Koh}Mg`#uGPW8*mr&Oa)vcmq=N0EKEN; z&Ru>bL$9zQ@L6@$lKE=ZmST`88ZUzR=QR*YIFw>v>e;F5#n$i=i2(=tZYo$51xgA8 zu!=GbrKuvkSCu?Sf;&q@$_>zXlRQd9=-|LV_a>z~I)QEfvVX^fQA zz9lTXdvr-emo~0^_RjNT-8HeM7U^klCPsmi03RRS^;se6@Im$G$XRI&7@ipUnf^`< zz*LYRg7e5PFshGW+37|gAeRLZuTQOv*oO2%h|3ZdA((h0A?d7@d)M}Yw@>-eIJ zsaE|v>^S?rI%5l^#M6dghCCLT;3Or>Bw^F;V#h|Gk^K_nL z1K%U^DC#CH;N3w=(WC!JH~^Jrx{w2)!yE%}{0{iwcok~X4@tAA!F-eWv^Q%piya8r z?F%!fr*Nw6U|azbIH^SxrS-T4VytjOAE@YaLwkyQaJAv3O4#k|qev7oV31bU=V_U6 zkTQPFLZ^4s6LyO_(46K{MIq0_rpA%ZBN$MQMp+OO`@E}@Hx82)@#{Qys>>E5f>2$$ oDTwbtLFR1yJGZ*MA1b; Date: Tue, 2 Jul 2024 17:41:37 -0500 Subject: [PATCH 05/12] add more tests Co-Authored-By: pluckyswan <96704946+pluckyswan@users.noreply.github.com> --- src/zip/zip.go | 2 +- src/zip/zip_test.go | 123 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 124 insertions(+), 1 deletion(-) diff --git a/src/zip/zip.go b/src/zip/zip.go index 4ffa770c..f95480f8 100644 --- a/src/zip/zip.go +++ b/src/zip/zip.go @@ -52,7 +52,6 @@ func (zipHandler ZipHandler) Unzip(zipFilePath string) error { return err } - slog.Info("Called unzip protected") zipReader, err := zipHandler.zipClient.OpenReader(zipFilePath) if err != nil { @@ -61,6 +60,7 @@ func (zipHandler ZipHandler) Unzip(zipFilePath string) error { } defer zipReader.Close() + // TODO - what if one file succeeds and another fails? for _, f := range zipReader.File { // TODO - should we warn or error if not encrypted? This would vary per customer if f.IsEncrypted() { diff --git a/src/zip/zip_test.go b/src/zip/zip_test.go index 5ff82121..7e59b097 100644 --- a/src/zip/zip_test.go +++ b/src/zip/zip_test.go @@ -2,6 +2,7 @@ package zip import ( "bytes" + "errors" "github.com/CDCgov/reportstream-sftp-ingestion/mocks" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" @@ -82,6 +83,128 @@ func Test_Unzip_FileIsNotProtected_UnzipsSuccessfully(t *testing.T) { assert.NoError(t, err) } +func Test_Unzip_UnableToGetPassword_ReturnsError(t *testing.T) { + os.Setenv("CA_DPH_ZIP_PASSWORD_NAME", "Test") + defer os.Unsetenv("CA_DPH_ZIP_PASSWORD_NAME") + defaultLogger := slog.Default() + defer slog.SetDefault(defaultLogger) + + buffer := &bytes.Buffer{} + slog.SetDefault(slog.New(slog.NewTextHandler(buffer, nil))) + + mockCredentialGetter := new(mocks.MockCredentialGetter) + + mockCredentialGetter.On("GetSecret", mock.Anything).Return("", errors.New("error")) + + zipHandler := ZipHandler{ + credentialGetter: mockCredentialGetter, + } + + err := zipHandler.Unzip("cheezburger") + + assert.NotContains(t, buffer.String(), "setting password") + assert.NotContains(t, buffer.String(), "file opened") + assert.Contains(t, buffer.String(), "Unable to get zip password") + assert.Error(t, err) +} + +func Test_Unzip_FailsToOpenReader_ReturnsError(t *testing.T) { + os.Setenv("CA_DPH_ZIP_PASSWORD_NAME", "Test") + defer os.Unsetenv("CA_DPH_ZIP_PASSWORD_NAME") + defaultLogger := slog.Default() + defer slog.SetDefault(defaultLogger) + + buffer := &bytes.Buffer{} + slog.SetDefault(slog.New(slog.NewTextHandler(buffer, nil))) + + mockCredentialGetter := new(mocks.MockCredentialGetter) + mockZipClient := new(MockZipClient) + + mockCredentialGetter.On("GetSecret", mock.Anything).Return("test123", nil) + + mockZipClient.On("OpenReader", mock.Anything).Return(&zip.ReadCloser{}, errors.New("error")) + + zipHandler := ZipHandler{ + credentialGetter: mockCredentialGetter, + zipClient: mockZipClient, + } + + err := zipHandler.Unzip("cheezburger") + + assert.NotContains(t, buffer.String(), "setting password") + assert.NotContains(t, buffer.String(), "file opened") + assert.Contains(t, buffer.String(), "Failed to open zip reader") + assert.Error(t, err) +} + +func Test_Unzip_FilePasswordIsWrong_ReturnsError(t *testing.T) { + os.Setenv("CA_DPH_ZIP_PASSWORD_NAME", "Test") + defer os.Unsetenv("CA_DPH_ZIP_PASSWORD_NAME") + defaultLogger := slog.Default() + defer slog.SetDefault(defaultLogger) + + buffer := &bytes.Buffer{} + slog.SetDefault(slog.New(slog.NewTextHandler(buffer, nil))) + + mockCredentialGetter := new(mocks.MockCredentialGetter) + mockZipClient := new(MockZipClient) + + mockCredentialGetter.On("GetSecret", mock.Anything).Return("test", nil) + + zipPath := filepath.Join("..", "mocks", "test_data", "passworded.zip") + zipReader, err := zip.OpenReader(zipPath) + + mockZipClient.On("OpenReader", mock.Anything).Return(zipReader, nil) + + zipHandler := ZipHandler{ + credentialGetter: mockCredentialGetter, + zipClient: mockZipClient, + } + + err = zipHandler.Unzip("cheezburger") + + assert.Contains(t, buffer.String(), "setting password") + assert.Contains(t, buffer.String(), "file opened") + assert.Contains(t, buffer.String(), "Failed to read file") + assert.Error(t, err) +} + +func Test_Unzip_UnzippedFileCannotBeUploaded_ReturnsError(t *testing.T) { + os.Setenv("CA_DPH_ZIP_PASSWORD_NAME", "Test") + defer os.Unsetenv("CA_DPH_ZIP_PASSWORD_NAME") + defaultLogger := slog.Default() + defer slog.SetDefault(defaultLogger) + + buffer := &bytes.Buffer{} + slog.SetDefault(slog.New(slog.NewTextHandler(buffer, nil))) + + mockCredentialGetter := new(mocks.MockCredentialGetter) + mockBlobHandler := new(mocks.MockBlobHandler) + mockZipClient := new(MockZipClient) + + mockCredentialGetter.On("GetSecret", mock.Anything).Return("test123", nil) + + zipPath := filepath.Join("..", "mocks", "test_data", "passworded.zip") + zipReader, err := zip.OpenReader(zipPath) + + mockZipClient.On("OpenReader", mock.Anything).Return(zipReader, nil) + + mockBlobHandler.On("UploadFile", mock.Anything, mock.Anything).Return(errors.New("error")) + + zipHandler := ZipHandler{ + credentialGetter: mockCredentialGetter, + blobHandler: mockBlobHandler, + zipClient: mockZipClient, + } + + err = zipHandler.Unzip("cheezburger") + + assert.Contains(t, buffer.String(), "setting password") + assert.Contains(t, buffer.String(), "file opened") + assert.Contains(t, buffer.String(), "Failed to upload file") + assert.Error(t, err) +} + type MockZipClient struct { mock.Mock } From 9bb53370903dac8212349dd3776ecb0897b4a7b2 Mon Sep 17 00:00:00 2001 From: Sylvie Date: Wed, 3 Jul 2024 12:18:22 -0500 Subject: [PATCH 06/12] Handle zip content errors more cleanly; upload SFTP-retrieved file based on file type; update tests Co-Authored-By: pluckyswan <96704946+pluckyswan@users.noreply.github.com> Co-Authored-By: jcrichlake <145698165+jcrichlake@users.noreply.github.com> Co-Authored-By: jherrflexion <118225331+jherrflexion@users.noreply.github.com> --- src/sftp/sftp.go | 18 ++++++++++++++--- src/utils/constants.go | 3 +++ src/zip/zip.go | 44 +++++++++++++++++++++++++++++++++++++++--- src/zip/zip_test.go | 8 ++++++-- 4 files changed, 65 insertions(+), 8 deletions(-) diff --git a/src/sftp/sftp.go b/src/sftp/sftp.go index fa39c117..451b90b0 100644 --- a/src/sftp/sftp.go +++ b/src/sftp/sftp.go @@ -11,6 +11,7 @@ import ( "io" "log/slog" "os" + "path/filepath" "strings" ) @@ -181,8 +182,13 @@ func (receiver *SftpHandler) copySingleFile(fileInfo os.FileInfo, index int, dir return } - // TODO - build a better path (unzip? import? how do we know?) - err = receiver.blobHandler.UploadFile(fileBytes, fileInfo.Name()) + var blobPath string + if strings.Contains(fileInfo.Name(), ".zip") { + blobPath = filepath.Join(utils.UnzipFolder, fileInfo.Name()) + } else { + blobPath = filepath.Join(utils.MessageStartingFolderPath, fileInfo.Name()) + } + err = receiver.blobHandler.UploadFile(fileBytes, blobPath) if err != nil { slog.Error("Failed to upload file", slog.Any(utils.ErrorKey, err)) } @@ -203,7 +209,10 @@ func (receiver *SftpHandler) copySingleFile(fileInfo os.FileInfo, index int, dir return } - _ = zipHandler.Unzip(fileInfo.Name()) + err = zipHandler.Unzip(fileInfo.Name()) + if err != nil { + slog.Error("Failed to unzip file", slog.Any(utils.ErrorKey, err)) + } //delete file from local filesystem err = os.Remove(fileInfo.Name()) @@ -211,6 +220,9 @@ func (receiver *SftpHandler) copySingleFile(fileInfo os.FileInfo, index int, dir slog.Error("Failed to remove file", slog.Any(utils.ErrorKey, err), slog.String("name", fileInfo.Name())) } + // TODO - currently the zip file stays in the `unzip` folder regardless of success, failure, or partial failure. + // Do we want to move the zip somewhere if done? + } } diff --git a/src/utils/constants.go b/src/utils/constants.go index c5171e61..b3da47cf 100644 --- a/src/utils/constants.go +++ b/src/utils/constants.go @@ -15,6 +15,9 @@ const SuccessFolder = "success" // we receive a failure response from ReportStream const FailureFolder = "failure" +// Zip files are placed in this folder after being retrieved from an external SFTP site +const UnzipFolder = "unzip" + // In read_and_send, move files to the `FailureFolder` when we get the below response from ReportStream const ReportStreamNonTransientFailure = "400" diff --git a/src/zip/zip.go b/src/zip/zip.go index f95480f8..a7afd263 100644 --- a/src/zip/zip.go +++ b/src/zip/zip.go @@ -9,6 +9,7 @@ import ( "io" "log/slog" "os" + "path/filepath" ) type ZipHandler struct { @@ -17,6 +18,11 @@ type ZipHandler struct { zipClient ZipClient } +type FileError struct { + Filename string + ErrorMessage string +} + func NewZipHandler() (ZipHandler, error) { blobHandler, err := storage.NewAzureBlobHandler() if err != nil { @@ -43,6 +49,11 @@ func NewZipHandler() (ZipHandler, error) { // TODO - check storage size/costs on the container // TODO - update CA password after deploy per env // TODO - check on visibility timeout for messages + +// Unzip opens a zip file (applying a password if necessary) and uploads each file within it to the `import` folder +// to begin processing. It collects any errors with individual subfiles and uploads that information as well. An error +// is only returned from the function when we cannot handle the main zip file for some reason or have failed to upload +// the error list about the contents func (zipHandler ZipHandler) Unzip(zipFilePath string) error { secretName := os.Getenv("CA_DPH_ZIP_PASSWORD_NAME") zipPassword, err := zipHandler.credentialGetter.GetSecret(secretName) @@ -60,6 +71,7 @@ func (zipHandler ZipHandler) Unzip(zipFilePath string) error { } defer zipReader.Close() + var errorList []FileError // TODO - what if one file succeeds and another fails? for _, f := range zipReader.File { // TODO - should we warn or error if not encrypted? This would vary per customer @@ -71,7 +83,8 @@ func (zipHandler ZipHandler) Unzip(zipFilePath string) error { fileReader, err := f.Open() if err != nil { slog.Error("Failed to open file", slog.Any(utils.ErrorKey, err)) - return err + errorList = append(errorList, FileError{Filename: f.Name, ErrorMessage: err.Error()}) + continue } defer fileReader.Close() @@ -80,13 +93,38 @@ func (zipHandler ZipHandler) Unzip(zipFilePath string) error { buf, err := io.ReadAll(fileReader) if err != nil { slog.Error("Failed to read file", slog.Any(utils.ErrorKey, err)) - return err + errorList = append(errorList, FileError{Filename: f.Name, ErrorMessage: err.Error()}) + continue } - err = zipHandler.blobHandler.UploadFile(buf, utils.MessageStartingFolderPath+"/"+f.FileInfo().Name()) + // After processing, move zip file somewhere? Is this different if partially vs fully vs 0 successful? + err = zipHandler.blobHandler.UploadFile(buf, filepath.Join(utils.MessageStartingFolderPath, f.FileInfo().Name())) if err != nil { slog.Error("Failed to upload file", slog.Any(utils.ErrorKey, err)) + errorList = append(errorList, FileError{Filename: f.Name, ErrorMessage: err.Error()}) + continue + } + } + // Upload error info if any + err = zipHandler.uploadErrorList(zipFilePath, errorList, err) + if err != nil { + return err + } + + return nil +} + +func (zipHandler ZipHandler) uploadErrorList(zipFilePath string, errorList []FileError, err error) error { + if len(errorList) > 0 { + fileContents := "" + for _, fileError := range errorList { + fileContents += fileError.Filename + ": " + fileError.ErrorMessage + "\n" + } + + err = zipHandler.blobHandler.UploadFile([]byte(fileContents), filepath.Join(utils.FailureFolder, zipFilePath)) + if err != nil { + slog.Error("Failed to upload failure file", slog.Any(utils.ErrorKey, err)) return err } } diff --git a/src/zip/zip_test.go b/src/zip/zip_test.go index 7e59b097..d205aeef 100644 --- a/src/zip/zip_test.go +++ b/src/zip/zip_test.go @@ -137,7 +137,7 @@ func Test_Unzip_FailsToOpenReader_ReturnsError(t *testing.T) { assert.Error(t, err) } -func Test_Unzip_FilePasswordIsWrong_ReturnsError(t *testing.T) { +func Test_Unzip_FilePasswordIsWrong_UploadsErrorDocument(t *testing.T) { os.Setenv("CA_DPH_ZIP_PASSWORD_NAME", "Test") defer os.Unsetenv("CA_DPH_ZIP_PASSWORD_NAME") defaultLogger := slog.Default() @@ -148,6 +148,7 @@ func Test_Unzip_FilePasswordIsWrong_ReturnsError(t *testing.T) { mockCredentialGetter := new(mocks.MockCredentialGetter) mockZipClient := new(MockZipClient) + mockBlobHandler := new(mocks.MockBlobHandler) mockCredentialGetter.On("GetSecret", mock.Anything).Return("test", nil) @@ -155,18 +156,21 @@ func Test_Unzip_FilePasswordIsWrong_ReturnsError(t *testing.T) { zipReader, err := zip.OpenReader(zipPath) mockZipClient.On("OpenReader", mock.Anything).Return(zipReader, nil) + mockBlobHandler.On("UploadFile", mock.Anything, mock.Anything).Return(nil) zipHandler := ZipHandler{ credentialGetter: mockCredentialGetter, zipClient: mockZipClient, + blobHandler: mockBlobHandler, } err = zipHandler.Unzip("cheezburger") + mockBlobHandler.AssertCalled(t, "UploadFile", mock.Anything, "failure/cheezburger") assert.Contains(t, buffer.String(), "setting password") assert.Contains(t, buffer.String(), "file opened") assert.Contains(t, buffer.String(), "Failed to read file") - assert.Error(t, err) + assert.NoError(t, err) } func Test_Unzip_UnzippedFileCannotBeUploaded_ReturnsError(t *testing.T) { From 4993a4583a885fb08e8fd490c766718bb500487d Mon Sep 17 00:00:00 2001 From: Sylvie Date: Wed, 3 Jul 2024 12:28:56 -0500 Subject: [PATCH 07/12] do a little cleanup Co-Authored-By: pluckyswan <96704946+pluckyswan@users.noreply.github.com> Co-Authored-By: jcrichlake <145698165+jcrichlake@users.noreply.github.com> Co-Authored-By: jherrflexion <118225331+jherrflexion@users.noreply.github.com> --- src/sftp/sftp.go | 14 +++++++------- src/zip/zip.go | 8 +++----- src/zip/zip_test.go | 4 ++-- 3 files changed, 12 insertions(+), 14 deletions(-) diff --git a/src/sftp/sftp.go b/src/sftp/sftp.go index 451b90b0..87dc716c 100644 --- a/src/sftp/sftp.go +++ b/src/sftp/sftp.go @@ -55,8 +55,7 @@ func NewSftpHandler() (*SftpHandler, error) { if err != nil { return nil, err } - slog.Info("Creating SSH client") - //TODO: Figure out if the ssh client config and the creation of the sftp client should go inside it's own function + config := &ssh.ClientConfig{ User: os.Getenv("SFTP_USER"), Auth: []ssh.AuthMethod{ @@ -134,10 +133,7 @@ func (receiver *SftpHandler) Close() { } func (receiver *SftpHandler) CopyFiles() { - // TODO - use "files" for readDir for now, but maybe replace with an env var for whatever directory - // we should start in - this should also be used in sftpClient.open below directory := "files" - //readDir using sftp client fileInfos, err := receiver.sftpClient.ReadDir(directory) if err != nil { slog.Error("Failed to read directory ", slog.Any(utils.ErrorKey, err)) @@ -153,7 +149,10 @@ func (receiver *SftpHandler) CopyFiles() { /* Eventually: - - have per-customer config, which contains things like how to connect to external servers (if any) and when, plus blob storage folder name + - have per-customer config, which contains things like how to connect to external servers (if any) and when, + plus blob storage folder name + - replace `files` hard-coded above with a per-customer value for e.g. `sftp_starting_folder` or similar (where + we go on their external SFTP server to retrieve files) - pass customer info to SFTP client, so we know whose files these are/what creds to use - since we have customer info, can use that to build destination path for upload - have a type or enum or something for allowed destination subfolders? E.g. import, unzip, failure, success, etc. @@ -161,6 +160,8 @@ func (receiver *SftpHandler) CopyFiles() { } +// copySingleFile moves a single file from an external SFTP server to our blob storage. Zip files go to an `unzip` +// folder and then we call the zipHandler.Unzip. Other files go to `import` to begin processing func (receiver *SftpHandler) copySingleFile(fileInfo os.FileInfo, index int, directory string) { slog.Info("Considering file", slog.String("name", fileInfo.Name()), slog.Int("number", index)) if fileInfo.IsDir() { @@ -222,7 +223,6 @@ func (receiver *SftpHandler) copySingleFile(fileInfo os.FileInfo, index int, dir // TODO - currently the zip file stays in the `unzip` folder regardless of success, failure, or partial failure. // Do we want to move the zip somewhere if done? - } } diff --git a/src/zip/zip.go b/src/zip/zip.go index a7afd263..c827b434 100644 --- a/src/zip/zip.go +++ b/src/zip/zip.go @@ -43,8 +43,6 @@ func NewZipHandler() (ZipHandler, error) { }, nil } -// TODO - refactor for tests? -// TODO - tests // TODO - move remaining items to future cards? // TODO - check storage size/costs on the container // TODO - update CA password after deploy per env @@ -72,7 +70,7 @@ func (zipHandler ZipHandler) Unzip(zipFilePath string) error { defer zipReader.Close() var errorList []FileError - // TODO - what if one file succeeds and another fails? + for _, f := range zipReader.File { // TODO - should we warn or error if not encrypted? This would vary per customer if f.IsEncrypted() { @@ -97,7 +95,6 @@ func (zipHandler ZipHandler) Unzip(zipFilePath string) error { continue } - // After processing, move zip file somewhere? Is this different if partially vs fully vs 0 successful? err = zipHandler.blobHandler.UploadFile(buf, filepath.Join(utils.MessageStartingFolderPath, f.FileInfo().Name())) if err != nil { @@ -115,6 +112,7 @@ func (zipHandler ZipHandler) Unzip(zipFilePath string) error { return nil } +// uploadErrorList takes a list of file-specific errors and uploads them to a single file named after the containing zip func (zipHandler ZipHandler) uploadErrorList(zipFilePath string, errorList []FileError, err error) error { if len(errorList) > 0 { fileContents := "" @@ -122,7 +120,7 @@ func (zipHandler ZipHandler) uploadErrorList(zipFilePath string, errorList []Fil fileContents += fileError.Filename + ": " + fileError.ErrorMessage + "\n" } - err = zipHandler.blobHandler.UploadFile([]byte(fileContents), filepath.Join(utils.FailureFolder, zipFilePath)) + err = zipHandler.blobHandler.UploadFile([]byte(fileContents), filepath.Join(utils.FailureFolder, zipFilePath+".txt")) if err != nil { slog.Error("Failed to upload failure file", slog.Any(utils.ErrorKey, err)) return err diff --git a/src/zip/zip_test.go b/src/zip/zip_test.go index d205aeef..718cec13 100644 --- a/src/zip/zip_test.go +++ b/src/zip/zip_test.go @@ -164,9 +164,9 @@ func Test_Unzip_FilePasswordIsWrong_UploadsErrorDocument(t *testing.T) { blobHandler: mockBlobHandler, } - err = zipHandler.Unzip("cheezburger") + err = zipHandler.Unzip("cheezburger.zip") - mockBlobHandler.AssertCalled(t, "UploadFile", mock.Anything, "failure/cheezburger") + mockBlobHandler.AssertCalled(t, "UploadFile", mock.Anything, "failure/cheezburger.zip.txt") assert.Contains(t, buffer.String(), "setting password") assert.Contains(t, buffer.String(), "file opened") assert.Contains(t, buffer.String(), "Failed to read file") From f478d2ac88ab08ba5d84e0cc465a7ac895ba3da7 Mon Sep 17 00:00:00 2001 From: Sylvie Date: Wed, 3 Jul 2024 14:14:43 -0500 Subject: [PATCH 08/12] close fileReader explicitly instead of using defer inside a loop Co-Authored-By: pluckyswan <96704946+pluckyswan@users.noreply.github.com> Co-Authored-By: jcrichlake <145698165+jcrichlake@users.noreply.github.com> Co-Authored-By: jherrflexion <118225331+jherrflexion@users.noreply.github.com> --- src/zip/zip.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/zip/zip.go b/src/zip/zip.go index c827b434..bcd8479c 100644 --- a/src/zip/zip.go +++ b/src/zip/zip.go @@ -84,12 +84,12 @@ func (zipHandler ZipHandler) Unzip(zipFilePath string) error { errorList = append(errorList, FileError{Filename: f.Name, ErrorMessage: err.Error()}) continue } - defer fileReader.Close() slog.Info("file opened", slog.Any("file", f)) buf, err := io.ReadAll(fileReader) if err != nil { + fileReader.Close() slog.Error("Failed to read file", slog.Any(utils.ErrorKey, err)) errorList = append(errorList, FileError{Filename: f.Name, ErrorMessage: err.Error()}) continue @@ -98,10 +98,12 @@ func (zipHandler ZipHandler) Unzip(zipFilePath string) error { err = zipHandler.blobHandler.UploadFile(buf, filepath.Join(utils.MessageStartingFolderPath, f.FileInfo().Name())) if err != nil { + fileReader.Close() slog.Error("Failed to upload file", slog.Any(utils.ErrorKey, err)) errorList = append(errorList, FileError{Filename: f.Name, ErrorMessage: err.Error()}) continue } + fileReader.Close() } // Upload error info if any err = zipHandler.uploadErrorList(zipFilePath, errorList, err) From 51debbfb3faa4d1fdb21e3e8c5d7873069556060 Mon Sep 17 00:00:00 2001 From: Sylvie Date: Wed, 3 Jul 2024 14:24:20 -0500 Subject: [PATCH 09/12] Update README.md Co-Authored-By: pluckyswan <96704946+pluckyswan@users.noreply.github.com> Co-Authored-By: jcrichlake <145698165+jcrichlake@users.noreply.github.com> Co-Authored-By: jherrflexion <118225331+jherrflexion@users.noreply.github.com> --- README.md | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 21e3459f..f5a2dc45 100644 --- a/README.md +++ b/README.md @@ -105,8 +105,13 @@ match your newly-created file 4. The app should now read this message and attempt to process it For the external SFTP call, we've set up a file in docker-compose that's copied to the local SFTP server. The service -then copies it to local Azurite. As of 6/28/24, nothing happens to this file after copy since it's not placed in -an `import` folder. +then copies it to local Azurite. You can add additional files by placing them in `localdata/data/sftp` before running +`docker-compose`. + +As of 7/3/24, when we copy a file from the local SFTP server, we try to unzip it +(using the password in `mock_credentials/mock_ca_dph_zip_password.txt` if it's protected). We then place the unzipped +files into the import folder, and if there are any errors, we upload an error file for the zip. If the original file is +not a zip, we just copy it into the import folder. #### Manual cloud testing To trigger file ingestion in a deployed environment, go to the `cdcrssftp{env}` storage account in the Azure portal. From 0c8a81897292f8b461a5776c28e95ebb739bd02e Mon Sep 17 00:00:00 2001 From: Sylvie Date: Wed, 3 Jul 2024 14:41:42 -0500 Subject: [PATCH 10/12] change mock zip client receiver to be a pointer Co-Authored-By: pluckyswan <96704946+pluckyswan@users.noreply.github.com> Co-Authored-By: jcrichlake <145698165+jcrichlake@users.noreply.github.com> --- src/zip/zip_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/zip/zip_test.go b/src/zip/zip_test.go index 718cec13..e217b7e0 100644 --- a/src/zip/zip_test.go +++ b/src/zip/zip_test.go @@ -213,7 +213,7 @@ type MockZipClient struct { mock.Mock } -func (mockZipClient MockZipClient) OpenReader(name string) (*zip.ReadCloser, error) { +func (mockZipClient *MockZipClient) OpenReader(name string) (*zip.ReadCloser, error) { args := mockZipClient.Called(name) return args.Get(0).(*zip.ReadCloser), args.Error(1) } From 97adcbd67186bd56acca58796ad1a8b2f41047a8 Mon Sep 17 00:00:00 2001 From: Sylvie Date: Fri, 5 Jul 2024 11:17:30 -0500 Subject: [PATCH 11/12] Refactor the unzip function to pull out handling-each-file and update some log statements Co-Authored-By: jcrichlake <145698165+jcrichlake@users.noreply.github.com> Co-Authored-By: pluckyswan <96704946+pluckyswan@users.noreply.github.com> --- src/zip/zip.go | 70 +++++++++++++++++++++++++++----------------------- 1 file changed, 38 insertions(+), 32 deletions(-) diff --git a/src/zip/zip.go b/src/zip/zip.go index bcd8479c..7ae97af1 100644 --- a/src/zip/zip.go +++ b/src/zip/zip.go @@ -53,6 +53,7 @@ func NewZipHandler() (ZipHandler, error) { // is only returned from the function when we cannot handle the main zip file for some reason or have failed to upload // the error list about the contents func (zipHandler ZipHandler) Unzip(zipFilePath string) error { + slog.Info("Preparing to unzip", slog.String("zipFilePath", zipFilePath)) secretName := os.Getenv("CA_DPH_ZIP_PASSWORD_NAME") zipPassword, err := zipHandler.credentialGetter.GetSecret(secretName) @@ -71,39 +72,9 @@ func (zipHandler ZipHandler) Unzip(zipFilePath string) error { var errorList []FileError + // loop over contents for _, f := range zipReader.File { - // TODO - should we warn or error if not encrypted? This would vary per customer - if f.IsEncrypted() { - slog.Info("setting password") - f.SetPassword(zipPassword) - } - - fileReader, err := f.Open() - if err != nil { - slog.Error("Failed to open file", slog.Any(utils.ErrorKey, err)) - errorList = append(errorList, FileError{Filename: f.Name, ErrorMessage: err.Error()}) - continue - } - - slog.Info("file opened", slog.Any("file", f)) - - buf, err := io.ReadAll(fileReader) - if err != nil { - fileReader.Close() - slog.Error("Failed to read file", slog.Any(utils.ErrorKey, err)) - errorList = append(errorList, FileError{Filename: f.Name, ErrorMessage: err.Error()}) - continue - } - - err = zipHandler.blobHandler.UploadFile(buf, filepath.Join(utils.MessageStartingFolderPath, f.FileInfo().Name())) - - if err != nil { - fileReader.Close() - slog.Error("Failed to upload file", slog.Any(utils.ErrorKey, err)) - errorList = append(errorList, FileError{Filename: f.Name, ErrorMessage: err.Error()}) - continue - } - fileReader.Close() + errorList = zipHandler.extractAndUploadSingleFile(f, zipPassword, errorList) } // Upload error info if any err = zipHandler.uploadErrorList(zipFilePath, errorList, err) @@ -114,6 +85,41 @@ func (zipHandler ZipHandler) Unzip(zipFilePath string) error { return nil } +func (zipHandler ZipHandler) extractAndUploadSingleFile(f *zip.File, zipPassword string, errorList []FileError) []FileError { + slog.Info("preparing to process file", slog.String("file name", f.Name)) + + // TODO - should we warn or error if not encrypted? This would vary per customer + if f.IsEncrypted() { + slog.Info("setting password for file", slog.String("file name", f.Name)) + f.SetPassword(zipPassword) + } + + fileReader, err := f.Open() + if err != nil { + slog.Error("Failed to open file", slog.String("file name", f.Name), slog.Any(utils.ErrorKey, err)) + errorList = append(errorList, FileError{Filename: f.Name, ErrorMessage: err.Error()}) + return errorList + } + defer fileReader.Close() + + buf, err := io.ReadAll(fileReader) + if err != nil { + slog.Error("Failed to read file", slog.String("file name", f.Name), slog.Any(utils.ErrorKey, err)) + errorList = append(errorList, FileError{Filename: f.Name, ErrorMessage: err.Error()}) + return errorList + } + + err = zipHandler.blobHandler.UploadFile(buf, filepath.Join(utils.MessageStartingFolderPath, f.FileInfo().Name())) + + if err != nil { + slog.Error("Failed to upload file", slog.String("file name", f.Name), slog.Any(utils.ErrorKey, err)) + errorList = append(errorList, FileError{Filename: f.Name, ErrorMessage: err.Error()}) + return errorList + } + slog.Info("uploaded file to blob for import", slog.String("file name", f.Name)) + return errorList +} + // uploadErrorList takes a list of file-specific errors and uploads them to a single file named after the containing zip func (zipHandler ZipHandler) uploadErrorList(zipFilePath string, errorList []FileError, err error) error { if len(errorList) > 0 { From 1e08614f58d8f374d5ab6b27b6f7359e62328465 Mon Sep 17 00:00:00 2001 From: Sylvie Date: Fri, 5 Jul 2024 11:24:25 -0500 Subject: [PATCH 12/12] update tests to match changes to logging Co-Authored-By: jcrichlake <145698165+jcrichlake@users.noreply.github.com> Co-Authored-By: pluckyswan <96704946+pluckyswan@users.noreply.github.com> Co-Authored-By: Samuel Aquino Co-Authored-By: Tiffini Johnson <86614374+tjohnson7021@users.noreply.github.com> --- src/zip/zip_test.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/zip/zip_test.go b/src/zip/zip_test.go index e217b7e0..924a8074 100644 --- a/src/zip/zip_test.go +++ b/src/zip/zip_test.go @@ -44,7 +44,7 @@ func Test_Unzip_FileIsPasswordProtected_UnzipsSuccessfully(t *testing.T) { err = zipHandler.Unzip("cheezburger") assert.Contains(t, buffer.String(), "setting password") - assert.Contains(t, buffer.String(), "file opened") + assert.Contains(t, buffer.String(), "preparing to process file") assert.NoError(t, err) } @@ -79,7 +79,7 @@ func Test_Unzip_FileIsNotProtected_UnzipsSuccessfully(t *testing.T) { err = zipHandler.Unzip("cheezburger") assert.NotContains(t, buffer.String(), "setting password") - assert.Contains(t, buffer.String(), "file opened") + assert.Contains(t, buffer.String(), "preparing to process file") assert.NoError(t, err) } @@ -103,7 +103,7 @@ func Test_Unzip_UnableToGetPassword_ReturnsError(t *testing.T) { err := zipHandler.Unzip("cheezburger") assert.NotContains(t, buffer.String(), "setting password") - assert.NotContains(t, buffer.String(), "file opened") + assert.NotContains(t, buffer.String(), "preparing to process file") assert.Contains(t, buffer.String(), "Unable to get zip password") assert.Error(t, err) } @@ -132,7 +132,7 @@ func Test_Unzip_FailsToOpenReader_ReturnsError(t *testing.T) { err := zipHandler.Unzip("cheezburger") assert.NotContains(t, buffer.String(), "setting password") - assert.NotContains(t, buffer.String(), "file opened") + assert.NotContains(t, buffer.String(), "preparing to process file") assert.Contains(t, buffer.String(), "Failed to open zip reader") assert.Error(t, err) } @@ -168,7 +168,7 @@ func Test_Unzip_FilePasswordIsWrong_UploadsErrorDocument(t *testing.T) { mockBlobHandler.AssertCalled(t, "UploadFile", mock.Anything, "failure/cheezburger.zip.txt") assert.Contains(t, buffer.String(), "setting password") - assert.Contains(t, buffer.String(), "file opened") + assert.Contains(t, buffer.String(), "preparing to process file") assert.Contains(t, buffer.String(), "Failed to read file") assert.NoError(t, err) } @@ -204,7 +204,7 @@ func Test_Unzip_UnzippedFileCannotBeUploaded_ReturnsError(t *testing.T) { err = zipHandler.Unzip("cheezburger") assert.Contains(t, buffer.String(), "setting password") - assert.Contains(t, buffer.String(), "file opened") + assert.Contains(t, buffer.String(), "preparing to process file") assert.Contains(t, buffer.String(), "Failed to upload file") assert.Error(t, err) }