Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #5296 +/- ##
============================================
+ Coverage 67.76% 68.34% +0.58%
- Complexity 0 26 +26
============================================
Files 358 453 +95
Lines 15594 17946 +2352
Branches 1735 2118 +383
============================================
+ Hits 10567 12266 +1699
- Misses 4500 5061 +561
- Partials 527 619 +92
☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
|
||
| router = APIRouter( | ||
| prefix="/fba", | ||
| dependencies=[Depends(asa_authentication_required), Depends(audit_asa)], |
There was a problem hiding this comment.
I think we need to remove these dependencies too
There was a problem hiding this comment.
This PR doesn't change or make the fba endpoints public, only the duplicate asa-go endpoints.
_=Depends(asa_authentication_required)) was removed from the endpoints below in this file because it was redundant
There was a problem hiding this comment.
Nevermind, saw the auth removals below and assumed this was the public API
| run: | | ||
| DEPLOY_VERSION="${GITHUB_RUN_ID}-${GITHUB_RUN_ATTEMPT}" MODULE_NAME=api SECOND_LEVEL_DOMAIN="apps.silver.devops.gov.bc.ca" VANITY_DOMAIN="${SUFFIX}-dev-psu.apps.silver.devops.gov.bc.ca" ENVIRONMENT="development" bash openshift/scripts/oc_deploy.sh ${SUFFIX} apply | ||
|
|
||
| - name: Deploy ASA Go API to Dev |
There was a problem hiding this comment.
Can ASA Go API deployment be another workflow that can be reused across dev and prod?
There was a problem hiding this comment.
Probably, I can try to put the bulk of it in an action
There was a problem hiding this comment.
Added an action to try to reduce some of the workflow duplication in: d65957b. I pulled the setting of SUFFIX out of the oc-setup action so it could more easily be used in with: workflow expressions, but it is also used for more than just openshift now so I thought this way was a little more explicit
| except HTTPException as exception: | ||
| logger.error("Could not validate the credential %s", exception) | ||
| except HTTPException: | ||
| # public audited routes intentionally allow anonymous access, so a missing bearer token |
There was a problem hiding this comment.
Is this change needed only for audited non logged in users?
There was a problem hiding this comment.
It isn't necessarily "needed", but if we still want to audit these endpoints, we get a lot of noise in the logs if the user doesn't have an idir (is public). If we do want to audit maybe we should ensure we aren't auditing if there is no idir....I'm not sure there would be any use in that
| if [ -n "${QUALIFIER}" ]; then | ||
| gwa publish-gateway "${CONFIG_PATH}" --qualifier "${QUALIFIER}" | ||
| else | ||
| gwa publish-gateway "${CONFIG_PATH}" |
There was a problem hiding this comment.
What cases are there where we wouldn't want a qualifier? Is there a risk that user error could unintentionally prune configs?
There was a problem hiding this comment.
You're right, there isn't really a case anymore where we wouldn't want a qualifier, this should always require one
| app: ${APP_NAME}-${SUFFIX}-asa-go | ||
| name: ${APP_NAME}-${SUFFIX}-asa-go | ||
| annotations: | ||
| image.openshift.io/triggers: |- |
There was a problem hiding this comment.
Do we want to use the deploy version idea we use in API?
There was a problem hiding this comment.
Hmm ya I could do that for consistency. We do have less race conditions/potential re-rollouts in this deployment because there's only 1 image, but I could change it to use deploy version to ensure there's only ever 1 rollout (potentially could be 2 now if image and template changed)
|
4 participants
fcmendpoints themselves can become public and be added to the ASA Go routerqualifier's to do soConfig refs
For local testing I recommend:
.env.developmentVITE_API_BASE_URLto port 8081closes #5249
Test Links:
Landing Page
MoreCast
Percentile Calculator
C-Haines
FireCalc
FireCalc bookmark
Auto Spatial Advisory (ASA)
HFI Calculator
SFMS Insights
Fire Watch
Weather Toolkit