Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

smoke_test: Fix Failure on Windows GitHub Workflow #571

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 8 additions & 5 deletions .github/actions/setup-dependencies-macos/action.yml
Original file line number Diff line number Diff line change
Expand Up @@ -12,15 +12,17 @@ inputs:
runs:
using: "composite"
steps:
- name: Setup virtualenv
- name: Virtual Environment Activation Definition - macOS or linux
shell: bash
run: |
python -m venv .venv
source .venv/bin/activate
run: >
echo "SCUBAGOGGLES_ACTIVATE_VENV=. .venv/bin/activate"
>> "$GITHUB_ENV"

- name: Install dependencies
- name: Setup virtual environment and install dependencies
shell: bash
run: |
python -m venv .venv
${{ env.SCUBAGOGGLES_ACTIVATE_VENV }}
python -m pip install .
pip install -r requirements.txt
pip install pytest
Expand All @@ -29,5 +31,6 @@ runs:
- name: Download OPA executable
shell: bash
run: |
${{ env.SCUBAGOGGLES_ACTIVATE_VENV }}
scubagoggles setup -m -nc -nd -d ~/scubagoggles -r ~/scubagoggles -c credentials.json
scubagoggles getopa -v ${{ inputs.opa-version }}
13 changes: 8 additions & 5 deletions .github/actions/setup-dependencies-windows/action.yml
Original file line number Diff line number Diff line change
Expand Up @@ -12,15 +12,17 @@ inputs:
runs:
using: "composite"
steps:
- name: Setup virtualenv
- name: Virtual Environment Activation Definition - Windows
shell: powershell
run: |
python -m venv .venv
.venv\Scripts\activate.ps1
run: >
echo "SCUBAGOGGLES_ACTIVATE_VENV=.venv\Scripts\activate.ps1"
>> "$Env:GITHUB_ENV"

- name: Install dependencies
- name: Setup virtual environment and install dependencies
shell: powershell
run: |
python -m venv .venv
${{ env.SCUBAGOGGLES_ACTIVATE_VENV }}
python -m pip install .
pip install -r requirements.txt
pip install pytest
Expand All @@ -29,5 +31,6 @@ runs:
- name: Download OPA executable
shell: powershell
run: |
${{ env.SCUBAGOGGLES_ACTIVATE_VENV }}
scubagoggles setup -m -nc -nd -d ~/scubagoggles -r ~/scubagoggles -c credentials.json
scubagoggles getopa -v ${{ inputs.opa-version }}
10 changes: 6 additions & 4 deletions .github/workflows/run_smoke_test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ on:
description: "Choose OPA version"
required: true
type: string
default: "v0.60.0"
default: "v1.0.0"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Recommend we pull in the latest OPA version rather than having to manually update default version of the input variable if we want latest. Then make this input optional but override the default if there's a diff.

OPA v1.0.0 is already out of date as OPA did a patch release to [OPA v1.0.1[(https://github.com/open-policy-agent/opa/releases/tag/v1.0.1) to address Go CVEs.

I did something similar on the ScubaGear side to check for latest OPA version updates

$LatestOPAVersion = Invoke-RestMethod -Uri "https://api.github.com/repos/open-policy-agent/opa/releases/latest" | Select-Object -ExpandProperty tag_name

Bash equivalent

latest_opa_version=$(curl -s https://api.github.com/repos/open-policy-agent/opa/releases/latest | grep '"tag_name"' | sed -E 's/.*"tag_name": "([^"]+)".*/\1/')

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This workflow now uses the latest OPA version, although this wasn’t the target of the PR/issue.


jobs:
configuration:
Expand All @@ -53,9 +53,9 @@ jobs:

# Default values for other events
else
operatingsystem_val="['macos-latest']"
operatingsystem_val="['windows-latest', 'macos-latest']"
pythonversion_val="['3.10']"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Other than in DownloadAndInstall.md where we indicate "v3.9 or higher", I don't see any references to a definitive minimum supported python version in the code. Recommend we move this out of the workflow in favor of a more maintainable location that doesn't require a code change when we decide to bump to v3.11, etc. Setting a GitHub secret as "MIN_PYTHON_VERSION"="3.10" may be a short-term alternative.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Support for Python 3.9 is dropping as of October this year, and we have another year for 3.10. I didn’t change the Python version used for this workflow (3.10). I think it’s a reasonable version to test with, considering 3.9 is going away. We can always look at other ways to have the workflow get the Python version to use. This PR is specific to the smoke test not working on Windows, and the issue is resolved with this PR.

opaversion_val="0.60.0"
opaversion_val="v1.0.0"
fi
echo "operating-system=$operatingsystem_val" >> "$GITHUB_OUTPUT"
echo "python-version=$pythonversion_val" >> "$GITHUB_OUTPUT"
Expand Down Expand Up @@ -106,4 +106,6 @@ jobs:
json: ${{ secrets.GWS_GITHUB_AUTOMATION_CREDS }}

- name: Run ScubaGoggles and check for correct output
run: pytest ./scubagoggles/Testing/Functional/SmokeTests/ -vvv --subjectemail="${{ secrets.GWS_SUBJECT_EMAIL }}" --customerdomain="${{ secrets.GWS_DOMAIN }}"
run: |
${{ env.SCUBAGOGGLES_ACTIVATE_VENV }}
pytest ./scubagoggles/Testing/Functional/SmokeTests/ -vvv --subjectemail="${{ secrets.GWS_SUBJECT_EMAIL }}" --customerdomain="${{ secrets.GWS_DOMAIN }}"
Loading