ADXT-89 feat(e2e): Introduce interface for environments and cross cloud-provi…

[KevinFairise2]

What does this PR do?

Create a new Env interface that must be implement by Environment struct specific for each Cloud Provider
It allows to have Cloud Agnostic function that can still access function that return values specific to a Cloud Provider. These methods should be described by the CloudEnv interface

Which scenarios this will impact?

Motivation

Additional Notes

[pducolin]

❓ question
What if these became static helpers taking Env as an argument?

[KevinFairise2]

Why not, I think both would work.
If we decide to keep it in the interface we'd need to have a BaseEnv struct that would implement these, because I do not think these methods depends on the cloud provider

[KevinFairise2]

Actually all these methods are defined in CommonEnvironment that is embedded in Environment so we do not need to define these several times.
To me it is easier to use these functions directly as method of the environment. Did you have anything in mind that would make it better if we change it to static helpers?

[pducolin]

Thinking about them, I ask how much they depend on the environment state.

Looking at one of the implementation, those functions use the environment only to log, and the logger is in the environment context.

test-infra-definitions/common/config/environment.go

Lines 245 to 256 in e5e1a13

	func (e *CommonEnvironment) GetBoolWithDefault(config *sdkconfig.Config, paramName string, defaultValue bool) bool {
	val, err := config.TryBool(paramName)
	if err == nil {
	return val
	}
	if !errors.Is(err, sdkconfig.ErrMissingVar) {
	e.Ctx.Log.Error(fmt.Sprintf("Parameter %s not parsable, err: %v, will use default value: %v", paramName, err, defaultValue), nil)
	}
	return defaultValue
	}

We could pass the context, and skip the strong dependency with the environment, so that we could get any value wherever from the configuration

[KevinFairise2]

Actually they do not depend on the environment. That's why they are method defined on CommonEnvironment, (here) and every environment should embed `CommonEnvironment

[KevinFairise2]

I started to move from a method to a static function that would take the context as an argument:

func GetBoolWithDefault(ctx pulumi.Context, config *sdkconfig.Config, paramName string, defaultValue bool) bool {
 	val, err := config.TryBool(paramName) 
 	if err == nil { 
 		return val 
 	} 
  
 	if !errors.Is(err, sdkconfig.ErrMissingVar) { 
 		ctx.Log.Error(fmt.Sprintf("Parameter %s not parsable, err: %v, will use default value: %v", paramName, err, defaultValue), nil) 
}

But it would make a lot more changes in this PR and I feel like it make the function a bit less discoverable. Do you mind if we think about this other improvement in a follow-up PR?

[pducolin]

Sure! Looks like they need an interface that implements a Log, ok for having it later