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

Improvements to VPCs and RDS #814

Draft
wants to merge 60 commits into
base: master
Choose a base branch
from
Draft

Improvements to VPCs and RDS #814

wants to merge 60 commits into from

Conversation

fmartingr
Copy link
Contributor

@fmartingr fmartingr commented Sep 27, 2024

List of PRs merged

fmartingr and others added 10 commits September 4, 2024 16:26
#781)

* show proxy ip address in deployment info

* add ProxyInstanceCount and ServerURL configs

- `ProxyInstanceCount` will determine how many proxy instances should be
  deployed. Default to 0, can be 0 or 1.
- `ServerURL` determines the URL to use with the Mattermost client from
  the agent nodes. It takes precedence over the Site URL, proxy IP, etc.

* downgrade elasticsearch dependency

* ProxyInstanceCount default 0

* faq

* feat: add concurrency to configureAndRunAgents

* Better log tracing

* Update docs/faq.md

Co-authored-by: Alejandro García Montoro <[email protected]>

* SAML configuration

* ltkeycloak

* typos

* fixed slog calls

* updated mattermost-realm.json

* saml configuration

* increase errorsChan buffer

* revert: agent concurrency

* docs: updated faq with SiteURL mention

* NewPointer

* refactor terraform app config

* mlog, ctx, worker number

* debug log

* revert saml changes

* make assets

* go mod tidy

* revert back channel to size 2, return early

---------

Co-authored-by: Alejandro García Montoro <[email protected]>
* Update Pyroscope v1
Add support for goroutine and block profiles

* Unify service name for targets

* Update test
* MM-56605: Create results file and print to file

* Separate writter into its own function for later

* Mofify writeToFile. Passing the file object.

* implement 	multiWriter so I can double the writes and then send them to cosole as well as file.

* Replace file object with interface.

* - Remove separate function to create file
- Move file creatuin and writting to the file to Comparison function

* Draft unti test for PrintResults

* Make test runnable by using LoadTests and Status properly.

* - Solve missing use of interface in the initail code chnage
- Solve whitespace mismatch

* Change expected to allign with the output. This solves failing unit test.

* Address most several of the code review feedback point.

* Use existing file path that is already being provided to the function.

* Address linter complaint about use of `Sprintf` for strings that need no formating.

* Address review comments

* Remove line printing an error as it was redundant.

* Update another if block that redundantly prints and returns the same error.

* Implement missed correction from Alejandro. If block must return the correct var when returning the error.
Ensure this is not occuring on other lines I've added.
Without this line, the deployment fails because
it cannot do t.getAsset
* ltkeycloak: added flag to force migrate all users

* changed flag

* avoid logging and returning error

* modified comment

* remove duplicated restoreasset

* reverted condition to return early

* check for nil values

* use UpdateUserAuth

* rename variable name

* Update cmd/ltkeycloak/from_mattermost.go

Co-authored-by: Alejandro García Montoro <[email protected]>

---------

Co-authored-by: Alejandro García Montoro <[email protected]>
…e version (#813)

* customizable database engine version

* Use same VPC for all resources

* make assets

* removed engineversion validation

* wait for stdout

* use default subnet if not specified
@agnivade
Copy link
Member

Looks like the branch got messed up.

@fmartingr fmartingr changed the title Legal hold improvements to VPCs and RDS Improvements to VPCs and RDS Sep 27, 2024
fmartingr and others added 22 commits October 22, 2024 22:57
My editor wants to make the Terraform files pretty.
This field does no longer exist, we can remove it from the sample files
and documentation
The method set of a type does not contain the methods on its pointer.
For fmt.Sprintf and friends to use it when using the "%s" formatter, we
need the method to be defined in the plain type.
MakeSlice expects its first argument to be the type of a slice, not the
type of the underlying elements of a slice; i.e., we need to pass
`[]string`, not `string`.

Before this commit, we were passing the type of the slice's Elem(),
which is the type of the underlying elements.
Defaulting the size of each slice to 0, we make sure the slices are not
nil, but valid empty slices.
Iterate over all fields of the ClusterSubnetIDs struct that are slices
and actually check at least one is non-nil with length greater than 0
Now that we validate that var.cluster_subnet_ids.xyz's length is larger
than 0, we allow both:
1. Unset lists, which are indeed serialized as empty strings. The length
   of an empty string is still 0.
2. Empty lists, which are serialized as lists, but that are not equal to
   the empty string.

If we only check for the empty string, we lose 2.
We were checking for the length of the `cluster_subnet_ids` variable,
not the underlying `redis` or `database` lists, which are the ones we
later use in these definitions.
If we don't define the VPC, leaving ClusterVPCID empty or unset, the
data.aws_subnets.selected list ends up being empty, since we filter for
subnets that are contained in such VPC.

To fix this, we retrieve the default VPC, and use its ID to filter the
subnets to select if no VPC is explicitly set.
Similar to the VPC change, we filter the selected subnets by their
availability zone *only* if the `AWSAvailabilityZone` setting is
non-empty.
If the user doesn't specify a subnet for a resource, we don't need to
pick one for it, we just set the subnet_id to null.
Subnet groups require 2+ subnets

Co-authored-by: Felipe Martin <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants