Observability Builder Props for Addons

[Howlla]

Issue #, if available:~

Description of changes: Modify some addons to export their Type for props.
Modify observability-builder.ts to accept props for each addon

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

[elamaran11]

@Howlla Couple of questions for clarity

[Howlla]

To support Fargate cluster deployment we need to be able to pass props for coredns and cert-manager.
Those are the 2 addons that need props in the argument. The rest props were exposed for future use cases that may require configuration values to be passed.

We can remove the export type from coredns and adot but then customers would have to define type again locally in their code

[elamaran11]

LGTM. @shapirov103 Please check and run e2e

[elamaran11]

LGTM

[shapirov103]

@Howlla definitely a needed change!

The approach is a bit overloaded from the software engineering perspective and does not align well with the builder pattern which is supposed to abstract the complexity to build the object.

How about this style to enable prop overrides:

     ObservabilityBuilder.builder()
            .account(account)
            .region(region)
            .withAddOnProps(ObservabilityBuilder.ADDON.CoreDns, myCoreDnsProps)
            .withAddOnProps(ObservabilityBuilder.ADDON.CertManager, myCertManagerProps)
            .enableNativePatternAddOns()
            .addOns(...addOns)
            .build(scope, stackId);

Please let me know if you have any questions.

[Howlla]

hi @shapirov103
Could you help me figure out how to properly type this function?

i tried the following way

 public withAddOnProps(addon:addons.HelmAddOn | addons.CoreAddOn, props: addons.CoreAddOnProps | addons.HelmAddOnProps ){
        return this.addOns(
            new addon(props)
        )
    }

but i get the following error

This expression is not constructable.
Not all constituents of type 'CoreAddOn | HelmAddOn' are constructable.
 Type 'CoreAddOn' has no construct signatures.
 ```

[shapirov103]

hi @shapirov103 Could you help me figure out how to properly type this function?

i tried the following way

 public withAddOnProps(addon:addons.HelmAddOn | addons.CoreAddOn, props: addons.CoreAddOnProps | addons.HelmAddOnProps ){
        return this.addOns(
            new addon(props)
        )
    }

but i get the following error

This expression is not constructable.
Not all constituents of type 'CoreAddOn | HelmAddOn' are constructable.
 Type 'CoreAddOn' has no construct signatures.

@Howlla I think the easiest approach is going to be something like:

class ObservabilityBuilder {
   
   private certManagerProps: CertManagerAddOnProps;
   private awsLoadbalancerProps: AwsLoadBalancerAddOnProps;
 ...

  public withCertManagerProps(props: CertManagerAddOnProps) {
     this.certManagerProps = props; // actually deep clone here to avoid side effects
  }

  private withAwsLoadBalancerProps(props: AwsLoadBalancerAddOnProps) {
    ...
  }
  
 
}


ObservabilityBuilder.builder()
    .withCertManagerProps( {someprops} )
    .withAwsLoadBalancerProps({some props})
    .enable...()

The downside is that for each addon there is an override method, but it makes it easy for the users to consume.

[Howlla]

hi @shapirov103 Could you help me figure out how to properly type this function?
i tried the following way

 public withAddOnProps(addon:addons.HelmAddOn | addons.CoreAddOn, props: addons.CoreAddOnProps | addons.HelmAddOnProps ){
        return this.addOns(
            new addon(props)
        )
    }

but i get the following error

This expression is not constructable.
Not all constituents of type 'CoreAddOn | HelmAddOn' are constructable.
 Type 'CoreAddOn' has no construct signatures.

@Howlla I think the easiest approach is going to be something like:

class ObservabilityBuilder {
   
   private certManagerProps: CertManagerAddOnProps;
   private awsLoadbalancerProps: AwsLoadBalancerAddOnProps;
 ...

  public withCertManagerProps(props: CertManagerAddOnProps) {
     this.certManagerProps = props; // actually deep clone here to avoid side effects
  }

  private withAwsLoadBalancerProps(props: AwsLoadBalancerAddOnProps) {
    ...
  }
  
 
}


ObservabilityBuilder.builder()
    .withCertManagerProps( {someprops} )
    .withAwsLoadBalancerProps({some props})
    .enable...()

The downside is that for each addon there is an override method, but it makes it easy for the users to consume.

I have made the changes as per your recommendation Mikhail. Will require slight modification to the cdk-eks-observability-accelerator for these files.

[shapirov103]

@Howlla Great work and thank you for address the kubeproxy and adot addon configurability.

[shapirov103]

@elamaran11 I put my approval on this, looks like it is pending requested changes from your side, please take a look.

[elamaran11]

LGTM