feat(ingress): aws alb

[Hunter-Thompson]

TODO:

• Add tests

Summary

Add option for AWS ALB Ingress

See:

1. https://kubernetes-sigs.github.io/aws-load-balancer-controller/v2.4/
2. https://kubernetes-sigs.github.io/aws-load-balancer-controller/v2.4/guide/ingress/annotations/

Issue Link

Fixes #293

API

apiVersion: installation.mattermost.com/v1beta1
kind: Mattermost
metadata:
  name: mm-example-full                         # Chose the desired name
spec:
  size: 5000users                               # Adjust to your requirements
  awsLoadBalancerController:
    enabled: true
    hosts: 
      - hostName: 'asd.example.com'
    certificateARN: 'asdasd'
    internetFacing: true
  version: 6.0.1
  licenseSecret: ""  

Release Note

Adds an option to configure an AWS ALB instead of the default Nginx Ingress

[mattermod]

Hello @Hunter-Thompson,

Thanks for your pull request! A Core Committer will review your pull request soon. For code contributions, you can learn more about the review process here.

Per the Mattermost Contribution Guide, we need to add you to the list of approved contributors for the Mattermost project.

Please help complete the Mattermost contribution license agreement?
Once you have signed the CLA, please comment with /check-cla and confirm that the CLA check is green.

This is a standard procedure for many open source projects.

Please let us know if you have any questions.

We are very happy to have you join our growing community! If you're not yet a member, please consider joining our Contributors community channel to meet other contributors and discuss new opportunities with the core team.

[Hunter-Thompson]

/check-cla

[mattermod]

This PR has been automatically labelled "stale" because it hasn't had recent activity.
A core team member will check in on the status of the PR to help with questions.
Thank you for your contribution!

[gabrieljackson]

Hey @Hunter-Thompson, just checking in to see if you are still working on this PR. We have been keeping an eye on it while you have had it opened as a draft. Let us know if you want to chat about anything regarding this functionality. Thanks!

[Hunter-Thompson]

@gabrieljackson went away for awhile. Il be working on it this week.

Quick question, has apis/mattermost/v1alpha1 been deprecated in favor of apis/mattermost/v1beta1?

Do I have to make changes in both places or is v1beta1 fine?

[gabrieljackson]

Hey @Hunter-Thompson, yes only updating v1beta is fine here. v1alpha is deprecated as you mentioned.

Let us know if you need any additional support with this.

[Hunter-Thompson]

@gabrieljackson could you have a look at the API and let me know if youd like changes? If no changes are required - Il start writing unit tests.

[Szymongib]

Thanks for the PR!

Regarding the API, since it impacts both creation of service and ingress, I suppose it makes sense to keep it as a separate spec field. I am a bit worried about complexity implications, but it does look like a useful feature.

[gabrieljackson]

@Hunter-Thompson let us know if you need any support making the change that @Szymongib proposed. Thanks!

[Hunter-Thompson]

@gabrieljackson I've made the changes proposed. If the API looks good, I can start testing in my local Kubernetes cluster and also write unit tests.

[fmartingr]

Hey @Hunter-Thompson, thanks a lot for your contribution! It took me a while to get up to speed with everything so apologies for the delayed response!

I've reviewed your contribution and it's good to go for the most part, but I've added some comments regarding naming conventions, some clauses I found confusing and testing coverage. Please check them out and reach out if you have any questions.

Again, thank you for your time on this! Greatly appreciated.

Edited: Actually though about this but didn't wrote it for whatever reason, so just dumping this here for the future: Just wondering since we're going to have one ingress only if we should encapsulate logic for ingresses in our own custom type, same as we do for other things. Right now logic is straightforward for the supported ones, but that would get crazy with time, so just throwing this idea out in the open.

[Hunter-Thompson]

@fmartingr, thanks for review. Ive pushed a new commit addressing some of the points. Il start working on better tests soon.

[fmartingr]

@fmartingr, thanks for review. Ive pushed a new commit addressing some of the points. Il start working on better tests soon.

Awesome, thanks! Just re-request my review or ping me once you're done so I can take a look :)

[Hunter-Thompson]

@fmartingr, thanks for review. Ive pushed a new commit addressing some of the points. Il start working on better tests soon.

Awesome, thanks! Just re-request my review or ping me once you're done so I can take a look :)

@fmartingr, I changed the tests a bit, and added a few new ones.

It covers:

1. disable aws lb and enable aws lb
2. check if resources are deleted properly after enabling and disabling

Could you review?

[fmartingr]

Hey @Hunter-Thompson, thanks for taking care of that! It looks good to me. @gabrieljackson I've re-requested your 👀 's here as well.

[Hunter-Thompson]

Did you get a chance to review @gabrieljackson?

[Hunter-Thompson]

bump

[fmartingr]

Hey @mirshahriar, since Gabe is pretty busy, would you mind taking a look at this?

[mirshahriar]

Hey @mirshahriar, since Gabe is pretty busy, would you mind taking a look at this?

I'll have a look today.

[mirshahriar]

I added a few comments.

[mirshahriar]

LGTM. Thank you for your time and work.

[gabrieljackson]

Thanks for working with us to get this ready to be merged! It will be included in the next operator release.