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

Avoid using absolute paths from the deploying machine in instance and table properties #267

Closed
patchwork01 opened this issue Oct 12, 2022 · 0 comments · Fixed by #570
Closed
Assignees
Milestone

Comments

@patchwork01
Copy link
Collaborator

patchwork01 commented Oct 12, 2022

There are several instance and table properties that get set to an absolute path of several files in the machine that originally deployed the instance. Here are those properties:

sleeper.tags.file
sleeper.table.properties
sleeper.table.schema.file

We could ensure these will be correct wherever they are used, instead of just on the original deployment machine.

The simplest way to achieve this seems to be to specify where those files should be in relation to the instance.properties file. This would only be used for working with the files locally, and wouldn't be relevant when they're in S3. The CDK would know where to look.

We'll update the CDK to look for the table properties and schema files in locations next to the instance properties file, and the tags file in a location relative to the instance properties file.

We'll need to update the scripts that put the instance and table properties in the scripts/generated folder, to put them in the right place for the CDK. That includes the pre-deployment script, and the connect to table script.

We need to update any scripts that reference these properties, to infer the new file location correctly.

We won't yet address the possibility of overwriting changes that were made in the admin client, potentially by someone else. We may add some conflict resolution behaviour in the future.

Options

Properties as CDK commands

We could have some properties be used as commands for the CDK, eg. some variant of sleeper.table.properties could provide local files to add new tables.

We'll want to model in code which properties are just for telling the CDK what to do, potentially referencing local files, and which are properties that we actually want to store in S3. We could also modify the property names to make this clearer, eg. sleeper.cdk.table.properties to specify local files and sleeper.tables to reference deployed tables.

CDK retrieves properties from S3

We could have the CDK retrieve the properties from S3 and apply them from there. We could have a separate CDK project that just deploys the properties, and maybe run a job in AWS to deploy the rest of the system.

Specify file system structure to find files locally

Instead of referencing the local files, we could enforce that they are stored locally in a specific place relative to the instance properties file. When the CDK runs, it can look there to find the files.

Property-specific details

Table properties files

The sleeper.table.properties property specifies local files for the table properties. When the instance properties are saved to S3 we could add a property that just says which tables are already deployed. This could also reference those tables' configuration buckets.

When you update an instance via CDK, we could determine the current tables based on the property declaring what's already deployed. We would need to ensure we're explicit about what we want to change, and avoid overwriting the existing table properties with what we have locally. Those should be changed with the admin client, rather than the CDK directly.

We could make sure that sleeper.table.properties is only ever accessed by the CDK, and only set it when we want to add a new table. The connectToTable.sh script would download the table properties, but not set that property to reference it. We would need to ensure that that property is removed if it is set.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants