Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add custom installation volume support #1092

Merged
merged 1 commit into from
Dec 23, 2024

Conversation

gabrieljackson
Copy link
Contributor

Kubernetes volumes can now be added to installation deployments. Volumes backed by secrets can be created, updated, and deleted after an installation has been created. The volume secret data is first stored in AWS Secrets Manager and then appended as a k8s secret in an installation update. Volumes are mounted in the specified mount path of the container which should support many cases where sensitive information is required at the filesystem level of Mattermost.

Fixes https://mattermost.atlassian.net/browse/CLD-8711

Add custom installation volume support

@mm-cloud-bot mm-cloud-bot added the release-note Denotes a PR that will be considered when it comes time to generate release notes. label Dec 17, 2024
Kubernetes volumes can now be added to installation deployments.
Volumes backed by secrets can be created, updated, and deleted
after an installation has been created. The volume secret data is
first stored in AWS Secrets Manager and then appended as a k8s
secret in an installation update. Volumes are mounted in the
specified mount path of the container which should support many
cases where sensitive information is required at the filesystem
level of Mattermost.
@gabrieljackson gabrieljackson force-pushed the custom-installation-volumes branch from b863210 to 2c1f9bd Compare December 18, 2024 00:07
@gabrieljackson gabrieljackson marked this pull request as ready for review December 18, 2024 00:34
@gabrieljackson gabrieljackson added 2: Dev Review Requires review by a developer 2: Infra Review Requires review by a SRE labels Dec 18, 2024
Copy link
Contributor

@nickmisasi nickmisasi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One small callout but overall LGTM. Nicely done!


oldState := installationDTO.State

err = createVolumeRequest.Apply(installationDTO.Installation)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need to validate anything with this? Even if as simple as checking for conflicts with current volume names?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, we definitely want to do one final validation to look for issues. If you trace out the Apply you will end up here were we run some final checks: https://github.com/mattermost/mattermost-cloud/pull/1092/files#diff-888e481ee6df2e28a4dbed85a127b1d24d0a6d7cbefbdda42792d926dc1ec70bR63

@gabrieljackson gabrieljackson added kind/feature Categorizes issue or PR as related to a new feature. and removed 2: Dev Review Requires review by a developer labels Dec 18, 2024
command.Flags().StringVar(&flags.installationID, "installation", "", "The id of the installation to create a volume for.")
command.Flags().StringVar(&flags.volumeName, "volume-name", "", "The name of the volume to create.")
command.Flags().StringVar(&flags.mountPath, "mount-path", "", "The container path to mount the volume in.")
command.Flags().BoolVar(&flags.readOnly, "read-only", true, "Whether the volume should be read only or not.")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need a read-only flag as a mount of type Secret is always read-only https://kubernetes.io/docs/concepts/storage/volumes/#secret

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch, but this is just prepping for the future when we may have other volume types.

@gabrieljackson gabrieljackson added 4: Reviews Complete All reviewers have approved the pull request and removed 2: Infra Review Requires review by a SRE labels Dec 23, 2024
@gabrieljackson gabrieljackson merged commit 5112293 into master Dec 23, 2024
5 checks passed
@gabrieljackson gabrieljackson deleted the custom-installation-volumes branch December 23, 2024 18:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
4: Reviews Complete All reviewers have approved the pull request kind/feature Categorizes issue or PR as related to a new feature. release-note Denotes a PR that will be considered when it comes time to generate release notes.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants