From 5f94bff83ace6961e7d1f1aaadf2018afb7ff1da Mon Sep 17 00:00:00 2001 From: Monique Rio Date: Wed, 29 Jan 2025 09:25:06 -0500 Subject: [PATCH 1/2] ci: updates gha workflows * removes trivvy image scanning * adds a specific branch to the update-dependencies PR --- .github/workflows/build-deploy-release.yaml | 15 --------------- .github/workflows/build-main.yaml | 15 --------------- .github/workflows/update-dependencies.yaml | 1 + 3 files changed, 1 insertion(+), 30 deletions(-) diff --git a/.github/workflows/build-deploy-release.yaml b/.github/workflows/build-deploy-release.yaml index 675b726..ad09414 100644 --- a/.github/workflows/build-deploy-release.yaml +++ b/.github/workflows/build-deploy-release.yaml @@ -15,21 +15,6 @@ jobs: dockerfile: Dockerfile secrets: inherit - #scan-image: - #needs: build-production - #runs-on: ubuntu-latest - #steps: - #- name: Run Trivy vulnerability scanner - #uses: aquasecurity/trivy-action@master - #with: - #image-ref: ghcr.io/mlibrary/${{ vars.IMAGE_NAME }}:${{ github.event.release.tag_name }} - #format: 'table' - #exit-code: '1' - #ignore-unfixed: true - #vuln-type: 'os,library' - #severity: 'CRITICAL,HIGH' - #skip-files: '/gems/ruby/3.2.0/gems/openid_connect-2.2.0/spec/mock_response/public_keys/private_key.pem' - deploy-production: needs: build-production name: Deploy to production diff --git a/.github/workflows/build-main.yaml b/.github/workflows/build-main.yaml index 2e8372e..cc1b1ca 100644 --- a/.github/workflows/build-main.yaml +++ b/.github/workflows/build-main.yaml @@ -16,21 +16,6 @@ jobs: dockerfile: Dockerfile secrets: inherit - #scan-image: - #needs: build-unstable - #runs-on: ubuntu-latest - #steps: - #- name: Run Trivy vulnerability scanner - #uses: aquasecurity/trivy-action@master - #with: - #image-ref: ${{ needs.build-unstable.outputs.image }} - #format: 'table' - #exit-code: '1' - #ignore-unfixed: true - #vuln-type: 'os,library' - #severity: 'CRITICAL,HIGH' - #skip-files: '/gems/ruby/3.2.0/gems/openid_connect-2.2.0/spec/mock_response/public_keys/private_key.pem' - deploy-unstable: needs: build-unstable name: Deploy to workshop diff --git a/.github/workflows/update-dependencies.yaml b/.github/workflows/update-dependencies.yaml index 89678f2..c9f1624 100644 --- a/.github/workflows/update-dependencies.yaml +++ b/.github/workflows/update-dependencies.yaml @@ -62,6 +62,7 @@ jobs: id: cpr uses: peter-evans/create-pull-request@v6 with: + branch: update-dependencies commit-message: "Update dependencies" title: ${{ env.PR_TITLE }} body-path: /tmp/pr_body.md From 14d62dea40f1394cc46afa9ca2f510a44c292677 Mon Sep 17 00:00:00 2001 From: Monique Rio Date: Wed, 29 Jan 2025 10:51:54 -0500 Subject: [PATCH 2/2] Adds structured logging for fines * Adds a custom JSON formmater for production logs * Adds adds more helpful logging data for diganosing fine problems * Adds color logging for development * Adds app_env and log_level Service keys --- lib/routes/fines_and_fees.rb | 11 +++-------- lib/services.rb | 33 ++++++++++++++++++++++++++++++++- models/fines/nelnet.rb | 1 - models/fines/receipt.rb | 19 ++++++++++++------- 4 files changed, 47 insertions(+), 17 deletions(-) diff --git a/lib/routes/fines_and_fees.rb b/lib/routes/fines_and_fees.rb index 3450bbe..7a1243e 100644 --- a/lib/routes/fines_and_fees.rb +++ b/lib/routes/fines_and_fees.rb @@ -11,24 +11,19 @@ post "/pay" do fines = Fines.for(uniqname: session[:uniqname]) total_sum = fines.total_sum.to_f - # 2024-06-06 This messages mrio when there's a fine to make it easier to determine if the fines bug is still happening. - begin - HTTParty.post(S.slack_url, headers: {"Content-type" => "application/json"}, body: {text: "Someone started a fine payment attempt in account"}.to_json) - rescue - S.logger.error("Couldn't send slack message") - end amount = (params["pay_in_full"] == "true") ? total_sum : params["partial_amount"].to_f if amount <= total_sum nelnet = Nelnet.new(amount_due: amount.to_currency) session["order_number"] = nelnet.order_number - S.logger.info("Fee payment attempt: order_number: #{nelnet.order_number}") + S.logger.info("fine_payment_start", message: "Fine payment started", order_number: nelnet.order_number) redirect nelnet.url else flash[:error] = "You don't need to overpay!!!" redirect "/fines-and-fees" end - rescue + rescue => e + S.logger.error("fine_payment_error", message: "Unable to redirect to payment website: #{e.detailed_message}") flash[:error] = "Error: We were unable to redirect you to the payment website. Please try again" redirect "/fines-and-fees" end diff --git a/lib/services.rb b/lib/services.rb index 98a7f33..1015789 100644 --- a/lib/services.rb +++ b/lib/services.rb @@ -21,4 +21,35 @@ ENV["APP_VERSION"] || "APP_VERSION" end -SemanticLogger.add_appender(io: S.log_stream, level: :info) unless ENV["APP_ENV"] == "test" +S.register(:log_level) do + ENV["DEBUG"] ? :debug : :info +end + +S.register(:app_env) do + ENV["APP_ENV"] || "development" +end + +class ProductionFormatter < SemanticLogger::Formatters::Json + # Leave out the pid + def pid + end + + # Leave out the timestamp + def time + end + + # Leave out environment + def environment + end + + # Leave out application (This would be Semantic Logger, which isn't helpful) + def application + end +end + +case S.app_env +when "production" + SemanticLogger.add_appender(io: S.log_stream, level: S.log_level, formatter: ProductionFormatter.new) +when "development" + SemanticLogger.add_appender(io: S.log_stream, level: S.log_level, formatter: :color) +end diff --git a/models/fines/nelnet.rb b/models/fines/nelnet.rb index 03cb0e4..bdb3bb3 100644 --- a/models/fines/nelnet.rb +++ b/models/fines/nelnet.rb @@ -9,7 +9,6 @@ def initialize(amount_due:, order_type: "UMLibraryCirc", timestamp: DateTime.timestamp, order_number: "#{SecureRandom.alphanumeric(4)}.#{timestamp}") - @payment_url = ENV.fetch("NELNET_PAYMENT_URL") @amount_due = amount_due.delete(".") @order_number = order_number diff --git a/models/fines/receipt.rb b/models/fines/receipt.rb index f466f3b..a3a0a8d 100644 --- a/models/fines/receipt.rb +++ b/models/fines/receipt.rb @@ -11,32 +11,37 @@ def initialize(payment:, balance:) def self.for(uniqname:, nelnet_params:, order_number:, is_valid: Nelnet.verify(nelnet_params)) payment = Payment.new(nelnet_params) + error_params = { + order_number: order_number, + nelent_params: nelnet_params + } if !is_valid - S.logger.error("Fine payment error: order number #{order_number} payment could not be validated.") + S.logger.error("fine_payment_error", message: "order number #{order_number} payment could not be validated.", **error_params) return ErrorReceipt.new("Your payment could not be validated. Your payment order number is: #{payment.order_number}") end - if !/Approved/.match?(nelnet_params["transactionResultMessage"]) + if !/approved/i.match?(nelnet_params["transactionResultMessage"]) + S.logger.error("fine_payment_error", message: "transaction message is not \"Approved\"", **error_params) return ErrorReceipt.new("There was an error processing your payment.
The error message is: #{nelnet_params["transactionResultMessage"]}
Your payment order number is: #{order_number}") end payment_verification = Fines.verify_payment(uniqname: uniqname, order_number: order_number) if payment_verification.instance_of?(AlmaError) - S.logger.error("Fine payment error: order_number: #{payment.order_number}; message: #{payment_verification.message}") + S.logger.error("fine_payment_error", message: "There was an Alma error: #{payment_verification.message}", **error_params) ErrorReceipt.new("There was an error in processing your payment.
Your payment order number is: #{payment.order_number}
Server error: #{payment_verification.message}
") elsif payment_verification[:has_order_number] - S.logger.error("Fine payment error: order number #{order_number} is already in Alma.") + S.logger.error("fine_payment_error", message: "order number is already in Alma.", **error_params) ErrorReceipt.new("Your payment order number, #{order_number}, is already in the fines database.") elsif payment_verification[:total_sum].to_f.to_s == "0.0" - S.logger.error("Fine payment error: order number #{order_number} tried to pay $0.00") + S.logger.error("fine_payment_error", message: "Tried to pay $0.00", **error_params) ErrorReceipt.new("You do not have a balance. Your payment order number is: #{order_number}.") else # has not already paid resp = Fines.pay(uniqname: uniqname, amount: payment.amount, order_number: order_number) if resp.code != 200 error = AlmaError.new(resp) - S.logger.error("Fine payment error: order number #{order_number}; message: #{error.message}") + S.logger.error("fine_payment_error", message: "Failed to apply payment to Alma: #{error.message}", **error_params) ErrorReceipt.new("#{error.message}
Your payment order number is: #{order_number}") else - S.logger.info("Fine payment success") + S.logger.info("fine_payment_success", message: "Fine payment success", order_number: order_number) Receipt.new(payment: payment, balance: resp.parsed_response["total_sum"]) end end