-
Notifications
You must be signed in to change notification settings - Fork 30
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
Support Console secrets in param
source variable resolution
#707
base: main
Are you sure you want to change the base?
Conversation
7317938
to
cf04325
Compare
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## main #707 +/- ##
==========================================
+ Coverage 50.31% 50.90% +0.59%
==========================================
Files 92 92
Lines 2572 2605 +33
==========================================
+ Hits 1294 1326 +32
- Misses 1278 1279 +1
☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should use more than just consoleServiceStage
and consoleService
parameters
const paramType = resolveConsoleParamType(paramData.path); | ||
if (!paramType) continue; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like this means that we would only resolve parameters that fall under the expected paths of consoleServiceStage
and consoleService
but I think we want to be able to resolve parameters that fall outside of that as well.
For example, if a customer sets global parameters (parameter with no path) then we would want that parameter to be resolved as well.
We should be able to just drop the paramType
logic and just resolve any parameters returned by the API since the logic deciding which parameters to use or not is already calculated there.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For example, if a customer sets global parameters (parameter with no path) then we would want that parameter to be resolved as well.
That's a very good point @Danwakeem but I think we need to raise it Linear, and discuss with @skierkowski.
Currently there's no concept of global params in Framework, there's just service level and service + stage + region level.
So question here is, how we should treat params with no path set?
Should they be treated same as service level (in such case configuration service level params will override it), or maybe they should take precedence over any configuration service level params?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Danwakeem see the resolution order we agreed on Linear, there's no concept of global params there (at this point)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah agreed, definitely need @skierkowski input on this 👍 I can follow up there.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if those global need to be supported, let's update the spec (agreed resolution order in linear)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think it is explicitly outlined in linear but it has been discussed in various tickets in linear so it would be good to get more explicit confirmation.
My understanding was that we should treat global parameters as a fallback to consoleService
. Again this is not explicitly stated though so would be good to get confirmation 😅
On hold, as first, changes to backend API need to be deployed to Console backend prod stage