fix: resolve infinite recursion in Provisioner.Class(), Type(), and Description()#483
Conversation
…escription() The Provisioner struct in the envprov package had three methods (Class, Type, Description) that called themselves unconditionally, causing a stack overflow crash whenever invoked. Replace recursive self-calls with correct static return values: - Class() returns default (matches the class checked in Match()) - Type() returns environment (matches the type checked in Match()) - Description() returns empty string (consistent with envVarResourceTracker) Add unit tests to verify the methods return expected values and prevent future regressions. Fixes score-spec#481 Signed-off-by: Siddhartha Singh <siddharthagithub0007@gmail.com>
|
I have successfully updated and synchronized the branch with the latest changes from the Could you please approve the workflows to run and review the updated changes whenever you get a chance? Thank you! |
|
Thanks @Siddhartha-singh01! For helping my review, could you please test and share the associated commands (before and after would be ideal):
Thanks! |
|
Hi @mathieu-benoit thanks for the feedback! and sir if you feel anything to ask to validate the pr just ask The environment provisioner is a builtin provisioner (registered programmatically in generate.go, not loaded from provisioner YAML files), so it doesn't appear in the output of score-compose provisioners list. However, I've tested the fix thoroughly here are the results: Before (on main) Infinite Recursion Bug func (p *Provisioner) Class() string { return p.Class() } // infinite recursion After (this PR) Fixed Unit Test
CLI Test with enviroment resource
Generated compose.yml output :
No crash, compose file generated successfully ✅ score-compose provisioners list :
The branch is also fully synced with the latest main . Thanks! |
|
Thanks for the details, what about the use of this one https://docs.score.dev/examples/resource-provisioners/community/environment/score-compose/cmd/dotenv/? |
|
Thanks for the link, @mathieu-benoit I see that the The builtin provisioner I've fixed specifically handles OS/shell environment variables via My fix ensures that even if multiple environment provisioners are present, the builtin one won't cause a stack overflow when the framework tries to inspect it. |
|
@chris-stephenson, can we have your review on this one please? Thanks! |
|
The Fix itself looks good - but I'm actually concerned about how the |
|
Thanks @chris-stephenson That’s a fair point regarding the "magic" in the envprov definition. Currently, it’s quite tightly coupled to the environment resource resolution. This fix ensures that the loader doesn't crash when multiple provisioners are present, but it doesn't yet solve the underlying "magic" of how they are prioritized or overridden. I'd love to help rethink that interface in a follow-up PR to make the provisioner registry more transparent. |
|
Yes, @Siddhartha-singh01 . While the |
|
Absolutely, I'd be happy to create that issue! I completely agree the current behavior where the built-in provisioner is implicitly wired to os.LookupEnv without any way for the developer to override it is too opaque. The environment resource type may be special in how it's handled internally, but the provisioner backing it should be an explicit, user-facing choice. I'm envisioning the issue along these lines: Problem: Today, when a Score file declares an environment resource, the resolution is hardcoded to OS environment variables. There's no configuration surface for a developer to say "use a .env file for local dev" or "use AWS SSM / HashiCorp Vault in production." This tight coupling makes the provisioner non-overridable and difficult to extend. Proposed direction: Introduce an explicit provisioner selection mechanism so developers can configure their preferred source per resource whether that's OS env vars, a .env file, a parameter store, or a custom provider. The built-in os.LookupEnv behavior could remain as the default, but it should be one option among many rather than the only path. I'll get the issue drafted and link it back here. Looking forward to collaborating on the design, @chris-stephenson ! |
|
@chris-stephenson Created the feature request in #488. Happy to pick that up once this fix is merged! |





Summary
Fixes #481
The
Provisionerstruct ininternal/provisioners/envprov/envprov.gohad three methods (Class(),Type(),Description()) that called themselves unconditionally, causing infinite recursion and a stack overflow crash whenever invoked.Changes
internal/provisioners/envprov/envprov.goClass()→ returns"default"(consistent with the class checked inMatch())Type()→ returns"environment"(consistent with the type checked inMatch())Description()→ returns""(consistent withenvVarResourceTracker)internal/provisioners/envprov/envprov_test.goclass returns default,type returns environment,description returns empty string) to verify correct behavior and prevent regressions.Testing
All existing and new tests pass: