Page MenuHomePhabricator
Differential D12324

[terraform] Replace all config options with terraform variables and remove sops
ClosedPublic
Actions

Authored by will on Jun 5 2024, 5:06 PM.
Tags
None
Referenced Files
F5773944: D12324.diff
Mon, Apr 14, 11:48 PM
Unknown Object (File)
Thu, Apr 3, 3:05 PM
Unknown Object (File)
Sun, Mar 30, 9:16 PM
Unknown Object (File)
Sun, Mar 30, 9:16 PM
Unknown Object (File)
Fri, Mar 28, 6:32 AM
Unknown Object (File)
Mar 6 2025, 5:59 AM
Unknown Object (File)
Mar 6 2025, 4:44 AM
Unknown Object (File)
Mar 6 2025, 2:04 AM
View All Files
Subscribers
ashoat
tomek

Details

Reviewers
bartek
varun
ashoat
Commits
rCOMMa413bd61cbd9: [terraform] Replace all config options with terraform variables and remove sops
Summary

Bartek pointed out that SOPS was overkill for self-hosting. Additionally, several configuration options required the user to make terraform changes.

In this diff, I introduce the usage of terraform variables to

  1. configure mariadb username/password that was previously configured through sops
  2. configure the aws region, subnets, and user ip

Depends on D12323

Test Plan

terraform apply and successfully connected mysql client to rds endpoint

Diff Detail

Repository
rCOMM Comm
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

will created this revision.Jun 5 2024, 5:06 PM
Herald added subscribers: tomek, ashoat. · View Herald TranscriptJun 5 2024, 5:06 PM
will retitled this revision from [terraform] Replace all config options with .tfvars and remove sops to [terraform] Replace all config options with terraform variables and remove sops.Jun 5 2024, 5:07 PM
will added inline comments.
services/terraform/self-host/aws_vpc.tf
9 ↗(On Diff #41000)

Changed to numbers to avoid possible confusion with the letters used by availability zones

Harbormaster completed remote builds in B29465: Diff 41000.Jun 5 2024, 5:23 PM
will requested review of this revision.Jun 5 2024, 5:23 PM
will edited the summary of this revision. (Show Details)EditedJun 5 2024, 5:49 PM
will added a parent revision: D12323: [terraform] remove usage of s3 bucket for terraform config.

Was thinking similar to the dev folder, we can create a simple run.sh script with:

terraform init

terraform apply -var-file="secrets.tfvars"
will added inline comments.Jun 5 2024, 5:52 PM
services/terraform/self-host/aws_vpc.tf
33 ↗(On Diff #41000)

32 as the mask means the block resolves to a single exact ip address

will added a reviewer: ashoat.Jun 5 2024, 5:52 PM
bartek added a comment.Jun 6 2024, 12:31 AM

Two comments here:

  • When using one of "default" filenames (I recall terraform.tfvars), you don't have to use the -var-file="secrets.tfvars" arg
  • It's a good practice to commit terraform.tfvars.example (while gitignoring actual terraform.tfvars) with example variables (usually required ones, that don't have defaults in variables.tf). This file can be copied and renamed by individual users.

What do you think?

ashoat resigned from this revision.Jun 6 2024, 10:20 AM

I like @bartek's suggestions. I'm probably a bad reviewer for Terraform stuff since I'm not familiar

will added a comment.Jun 6 2024, 11:46 AM
In D12324#350437, @bartek wrote:

Two comments here:

  • When using one of "default" filenames (I recall terraform.tfvars), you don't have to use the -var-file="secrets.tfvars" arg
  • It's a good practice to commit terraform.tfvars.example (while gitignoring actual terraform.tfvars) with example variables (usually required ones, that don't have defaults in variables.tf). This file can be copied and renamed by individual users.

What do you think?

Really good ideas. Adding to new next rebase and modified the notion doc to use terraform.tfvars
https://www.notion.so/commapp/Moving-local-Keyserver-MariaDB-data-to-a-Self-hosted-AWS-RDS-instance-ed4fd1b02f944e3ab1ea074c65f553f3

will updated this revision to Diff 41069.Jun 6 2024, 11:47 AM

review feedback

bartek accepted this revision.Jun 6 2024, 12:02 PM
This revision is now accepted and ready to land.Jun 6 2024, 12:02 PM
Harbormaster completed remote builds in B29496: Diff 41069.Jun 6 2024, 12:03 PM
will added a child revision: D12339: [terraform] Use default vpc, subnets, and internet gateway for self-host.Jun 6 2024, 8:37 PM
will updated this revision to Diff 41078.Jun 6 2024, 8:59 PM

to be removed but zones should be included in the terraform variables for this diff

will updated this revision to Diff 41081.Jun 6 2024, 9:12 PM

rebase

Harbormaster completed remote builds in B29505: Diff 41078.Jun 6 2024, 9:23 PM
Harbormaster completed remote builds in B29508: Diff 41081.Jun 6 2024, 9:30 PM
Closed by commit rCOMMa413bd61cbd9: [terraform] Replace all config options with terraform variables and remove sops (authored by will). · Explain WhyJun 7 2024, 11:10 AM
This revision was automatically updated to reflect the committed changes.
will added a commit: rCOMMa413bd61cbd9: [terraform] Replace all config options with terraform variables and remove sops.

Revision Contents
Changeset List

PathSize
services/
terraform/
self-host/
4 lines
6 lines
20 lines
12 lines
5 lines
6 lines
34 lines

Diff 41078

View Options

services/terraform/self-host/.gitignore

  • This file was added.
// @flow
import invariant from 'invariant';
import Alert from 'react-native/Libraries/Alert/Alert';
import {
sendReactionMessage,
sendReactionMessageActionTypes,
} from 'lib/actions/message-actions';
import type { BindServerCall, DispatchFunctions } from 'lib/utils/action-utils';
import type { TooltipRoute } from '../navigation/tooltip.react';
function onPressReact(
route:
| TooltipRoute<'TextMessageTooltipModal'>
| TooltipRoute<'MultimediaMessageTooltipModal'>,
dispatchFunctions: DispatchFunctions,
bindServerCall: BindServerCall,
) {
const messageID = route.params.item.messageInfo.id;
const threadID = route.params.item.threadInfo.id;
tomekUnsubmitted
Not Done
Inline Actions

This approach seems to be overly complicated. Can't we just write a function that receives messageID, threadID and reaction? This function can be named after an action that is performed, so react or sendReaction.

tomek: This approach seems to be overly complicated. Can't we just write a function that receives…
invariant(messageID, 'messageID should be set');
invariant(threadID, 'threadID should be set');
const callSendReactionMessage = bindServerCall(sendReactionMessage);
const reactionMessagePromise = (async () => {
try {
const result = await callSendReactionMessage(threadID, messageID, '👍');
return {
targetMessageID: messageID,
serverID: result.id,
threadID,
time: result.newMessageInfo.time,
interface: result.interface,
newMessageInfos: [result.newMessageInfo],
};
} catch (e) {
Alert.alert(
tomekUnsubmitted
Not Done
Inline Actions

It sounds risky to show one alert after every failed message. A user might react to a lot of messages in short period and a network issue could result in a lot of alerts being shown. Other apps sometimes choose to silently fail when a reaction fails.

tomek: It sounds risky to show one alert after every failed message. A user might react to a lot of…
ginsuAuthorUnsubmitted
Done
Inline Actions

Totally see your concern here! I spoke to @atul about this today, and we both agreed that we want this to be a temporary solution, but since reaction messages don't have retry behavior we think it is best to show some sort of feedback to the user in case something failed.

ginsu: Totally see your concern here! I spoke to @atul about this today, and we both agreed that we…
'Couldn’t send the reaction',
'Please try again later',
[{ text: 'OK' }],
{
cancelable: false,
tomekUnsubmitted
Not Done
Inline Actions

Is there a reason to set cancelable to false?

tomek: Is there a reason to set cancelable to false?
ginsuAuthorUnsubmitted
Done
Inline Actions

Alerts on Android can be dismissed by tapping outside of the alert box. It is disabled by default and can be enabled by providing an optional Options parameter with the cancelable property set to true. Here is more context

I also found the alert on report-utils to follow the same patern

ginsu: Alerts on Android can be dismissed by tapping outside of the alert box. It is disabled by…
},
);
throw e;
}
})();
dispatchFunctions.dispatchActionPromise(
sendReactionMessageActionTypes,
reactionMessagePromise,
);
}
export { onPressReact };
View Options

services/terraform/self-host/aws_db.tf

  • This file was added.
// @flow
import invariant from 'invariant';
import Alert from 'react-native/Libraries/Alert/Alert';
import {
sendReactionMessage,
sendReactionMessageActionTypes,
} from 'lib/actions/message-actions';
import type { BindServerCall, DispatchFunctions } from 'lib/utils/action-utils';
import type { TooltipRoute } from '../navigation/tooltip.react';
function onPressReact(
route:
| TooltipRoute<'TextMessageTooltipModal'>
| TooltipRoute<'MultimediaMessageTooltipModal'>,
dispatchFunctions: DispatchFunctions,
bindServerCall: BindServerCall,
) {
const messageID = route.params.item.messageInfo.id;
const threadID = route.params.item.threadInfo.id;
tomekUnsubmitted
Not Done
Inline Actions

This approach seems to be overly complicated. Can't we just write a function that receives messageID, threadID and reaction? This function can be named after an action that is performed, so react or sendReaction.

tomek: This approach seems to be overly complicated. Can't we just write a function that receives…
invariant(messageID, 'messageID should be set');
invariant(threadID, 'threadID should be set');
const callSendReactionMessage = bindServerCall(sendReactionMessage);
const reactionMessagePromise = (async () => {
try {
const result = await callSendReactionMessage(threadID, messageID, '👍');
return {
targetMessageID: messageID,
serverID: result.id,
threadID,
time: result.newMessageInfo.time,
interface: result.interface,
newMessageInfos: [result.newMessageInfo],
};
} catch (e) {
Alert.alert(
tomekUnsubmitted
Not Done
Inline Actions

It sounds risky to show one alert after every failed message. A user might react to a lot of messages in short period and a network issue could result in a lot of alerts being shown. Other apps sometimes choose to silently fail when a reaction fails.

tomek: It sounds risky to show one alert after every failed message. A user might react to a lot of…
ginsuAuthorUnsubmitted
Done
Inline Actions

Totally see your concern here! I spoke to @atul about this today, and we both agreed that we want this to be a temporary solution, but since reaction messages don't have retry behavior we think it is best to show some sort of feedback to the user in case something failed.

ginsu: Totally see your concern here! I spoke to @atul about this today, and we both agreed that we…
'Couldn’t send the reaction',
'Please try again later',
[{ text: 'OK' }],
{
cancelable: false,
tomekUnsubmitted
Not Done
Inline Actions

Is there a reason to set cancelable to false?

tomek: Is there a reason to set cancelable to false?
ginsuAuthorUnsubmitted
Done
Inline Actions

Alerts on Android can be dismissed by tapping outside of the alert box. It is disabled by default and can be enabled by providing an optional Options parameter with the cancelable property set to true. Here is more context

I also found the alert on report-utils to follow the same patern

ginsu: Alerts on Android can be dismissed by tapping outside of the alert box. It is disabled by…
},
);
throw e;
}
})();
dispatchFunctions.dispatchActionPromise(
sendReactionMessageActionTypes,
reactionMessagePromise,
);
}
export { onPressReact };
View Options

services/terraform/self-host/aws_vpc.tf

  • This file was added.
// @flow
import invariant from 'invariant';
import Alert from 'react-native/Libraries/Alert/Alert';
import {
sendReactionMessage,
sendReactionMessageActionTypes,
} from 'lib/actions/message-actions';
import type { BindServerCall, DispatchFunctions } from 'lib/utils/action-utils';
import type { TooltipRoute } from '../navigation/tooltip.react';
function onPressReact(
route:
| TooltipRoute<'TextMessageTooltipModal'>
| TooltipRoute<'MultimediaMessageTooltipModal'>,
dispatchFunctions: DispatchFunctions,
bindServerCall: BindServerCall,
) {
const messageID = route.params.item.messageInfo.id;
const threadID = route.params.item.threadInfo.id;
tomekUnsubmitted
Not Done
Inline Actions

This approach seems to be overly complicated. Can't we just write a function that receives messageID, threadID and reaction? This function can be named after an action that is performed, so react or sendReaction.

tomek: This approach seems to be overly complicated. Can't we just write a function that receives…
invariant(messageID, 'messageID should be set');
invariant(threadID, 'threadID should be set');
const callSendReactionMessage = bindServerCall(sendReactionMessage);
const reactionMessagePromise = (async () => {
try {
const result = await callSendReactionMessage(threadID, messageID, '👍');
return {
targetMessageID: messageID,
serverID: result.id,
threadID,
time: result.newMessageInfo.time,
interface: result.interface,
newMessageInfos: [result.newMessageInfo],
};
} catch (e) {
Alert.alert(
tomekUnsubmitted
Not Done
Inline Actions

It sounds risky to show one alert after every failed message. A user might react to a lot of messages in short period and a network issue could result in a lot of alerts being shown. Other apps sometimes choose to silently fail when a reaction fails.

tomek: It sounds risky to show one alert after every failed message. A user might react to a lot of…
ginsuAuthorUnsubmitted
Done
Inline Actions

Totally see your concern here! I spoke to @atul about this today, and we both agreed that we want this to be a temporary solution, but since reaction messages don't have retry behavior we think it is best to show some sort of feedback to the user in case something failed.

ginsu: Totally see your concern here! I spoke to @atul about this today, and we both agreed that we…
'Couldn’t send the reaction',
'Please try again later',
[{ text: 'OK' }],
{
cancelable: false,
tomekUnsubmitted
Not Done
Inline Actions

Is there a reason to set cancelable to false?

tomek: Is there a reason to set cancelable to false?
ginsuAuthorUnsubmitted
Done
Inline Actions

Alerts on Android can be dismissed by tapping outside of the alert box. It is disabled by default and can be enabled by providing an optional Options parameter with the cancelable property set to true. Here is more context

I also found the alert on report-utils to follow the same patern

ginsu: Alerts on Android can be dismissed by tapping outside of the alert box. It is disabled by…
},
);
throw e;
}
})();
dispatchFunctions.dispatchActionPromise(
sendReactionMessageActionTypes,
reactionMessagePromise,
);
}
export { onPressReact };
View Options

services/terraform/self-host/main.tf

  • This file was added.
// @flow
import invariant from 'invariant';
import Alert from 'react-native/Libraries/Alert/Alert';
import {
sendReactionMessage,
sendReactionMessageActionTypes,
} from 'lib/actions/message-actions';
import type { BindServerCall, DispatchFunctions } from 'lib/utils/action-utils';
import type { TooltipRoute } from '../navigation/tooltip.react';
function onPressReact(
route:
| TooltipRoute<'TextMessageTooltipModal'>
| TooltipRoute<'MultimediaMessageTooltipModal'>,
dispatchFunctions: DispatchFunctions,
bindServerCall: BindServerCall,
) {
const messageID = route.params.item.messageInfo.id;
const threadID = route.params.item.threadInfo.id;
tomekUnsubmitted
Not Done
Inline Actions

This approach seems to be overly complicated. Can't we just write a function that receives messageID, threadID and reaction? This function can be named after an action that is performed, so react or sendReaction.

tomek: This approach seems to be overly complicated. Can't we just write a function that receives…
invariant(messageID, 'messageID should be set');
invariant(threadID, 'threadID should be set');
const callSendReactionMessage = bindServerCall(sendReactionMessage);
const reactionMessagePromise = (async () => {
try {
const result = await callSendReactionMessage(threadID, messageID, '👍');
return {
targetMessageID: messageID,
serverID: result.id,
threadID,
time: result.newMessageInfo.time,
interface: result.interface,
newMessageInfos: [result.newMessageInfo],
};
} catch (e) {
Alert.alert(
tomekUnsubmitted
Not Done
Inline Actions

It sounds risky to show one alert after every failed message. A user might react to a lot of messages in short period and a network issue could result in a lot of alerts being shown. Other apps sometimes choose to silently fail when a reaction fails.

tomek: It sounds risky to show one alert after every failed message. A user might react to a lot of…
ginsuAuthorUnsubmitted
Done
Inline Actions

Totally see your concern here! I spoke to @atul about this today, and we both agreed that we want this to be a temporary solution, but since reaction messages don't have retry behavior we think it is best to show some sort of feedback to the user in case something failed.

ginsu: Totally see your concern here! I spoke to @atul about this today, and we both agreed that we…
'Couldn’t send the reaction',
'Please try again later',
[{ text: 'OK' }],
{
cancelable: false,
tomekUnsubmitted
Not Done
Inline Actions

Is there a reason to set cancelable to false?

tomek: Is there a reason to set cancelable to false?
ginsuAuthorUnsubmitted
Done
Inline Actions

Alerts on Android can be dismissed by tapping outside of the alert box. It is disabled by default and can be enabled by providing an optional Options parameter with the cancelable property set to true. Here is more context

I also found the alert on report-utils to follow the same patern

ginsu: Alerts on Android can be dismissed by tapping outside of the alert box. It is disabled by…
},
);
throw e;
}
})();
dispatchFunctions.dispatchActionPromise(
sendReactionMessageActionTypes,
reactionMessagePromise,
);
}
export { onPressReact };
View Options

services/terraform/self-host/providers.tf

Loading...
View Options

services/terraform/self-host/terraform.tfvars.example

Loading...
View Options

services/terraform/self-host/variables.tf

Loading...