Rework provider implementations to avoid environment_variables and make it so people don't have to edit

[DigitalFrontiersMedia]

Is there an existing issue for this?

• I have searched the existing issues

Is your feature request related to a problem?

Use of environment_variables in Providers overcomplicates configurations and can cause duplication of effort in fixing bugs associated with yaml parsing. Centralizing this feature to config.yaml, global_config, .env, etc, would be better. platform.yaml already works this way.

Describe your solution

To avoid problems with users currently using this feature and to prevent unexpected breaking changes, add a notification to users stating that environment_variables in Providers will be deprecated in a future release if we detect the use of environment_variables in the provider.

Describe alternatives

An alternative is to simply remove the environment_variables parsing code from provider.go along with the notification and go ahead with the breaking change without early warning to change. I feel this might not be the best way since bugs that have been fixed that were lodged against providers should be tested against platform and general config parsing/handling and apply known fixes where they may be needed prior to dropping it entirely.

Additional context

No response

[rfay]

I don't see any reason to deprecate the capability, but rather stop using it (update the examples to use real environment variables). There's no reason to break existing provider integrations.

[DigitalFrontiersMedia]

Oh. Okay, I misunderstood you. However, won't this create a functional disparity, e.g. spaces in variable names works in providers but doesn't in config.yaml, etc.? I obviously don't know the history and don't know if there are multiple bugs that have been lodged and fixed against one but not the other.

We can obviously kill this or alter it. I was just trying to follow up with what had been discussed.

[rfay]

Spaces should work in variable names elsewhere. I think there's a solved issue about this.

But the path here would be to

1. Make all the provider integrations work without edits/copies/etc
2. Remove the docs about the environment section in providers.

[DigitalFrontiersMedia]

and

3. Add a notification if provider environment_variables are detected

as you previously suggested?

[rfay]

I think that's a good idea. I was probably too aggressive last time we talked, huh.

[DigitalFrontiersMedia]

I'd be aggressive too if I'd had to deal with those failing docker containers for as long as you. Hahaha

But sounds like a plan. Will look to initiate this.

[DigitalFrontiersMedia]

Can you provide more guidance on exactly what was meant by:

• Make all the provider integrations work without edits/copies/etc

What should the warning message say? Here's a suggestion to start from:
"Avoid placing environment_variables in Provider files. They should be placed instead in config.yaml, global_config, .env, etc."
What changes should be made to provide the preferred verbiage?

Also, I was experimenting with this and added a test message for this on my local in Provider.go's Init function. And for convenience's sake tried the manual test from #4594 to see the test message on the ddev pull test -y --skip-import command. The message appeared twice, meaning init is called twice during ddev pull in this way. Is that a bug? If not, is there a better place to put this notice? Or should I try to use a flag or something to limit the message to only appear once?

[rfay]

Make all the provider integrations work without edits/copies/etc

DDEV should provide an acquia.yaml and pantheon.yaml that work just like the platform.yaml, not requiring edits to an example file. There should be checks to make sure the relevant environment variables are set.

Avoid placing environment_variables in Provider files. They should be placed instead in config.yaml, global_config, .env, etc.

I think it would be better to say "Instead use DDEV environment variable settings, see https://ddev.readthedocs.io/en/stable/users/extend/customization-extendibility/#providing-custom-environment-variables-to-a-container"

I was experimenting with this and added a test message

Yes, I would expect that the code currently tries to evaluate the (current) environment variable setup for each bash stanza, since that's where we need them.

[DigitalFrontiersMedia]

Hi Randy! Sorry for disappearing. Had a downturn in my finances and am still recovering. But at least now, on a cool project at Red Hat. I haven't forgotten you. I plan to start devoting some time back on here in the next few weeks. Congratulations on the award at DrupalCon, btw! :-) I was glad to see that!

[rfay]

Glad you're recovering and on a good project! Will look forward to having you back. Feel free to sign up for the contributor training at https://ddev.com/blog/contributor-training/

[rfay]

• fix: build new ddev-webserver to get fixed lagoon-sync, rework Acquia for #4760 #5519 reworks Acquia completely so no more editing is necessary.