Replace Airnode's short address with deployment ID #1577
Conversation
| .createHash('sha256') | ||
| .update([...cloudProviderHashElements(cloudProvider), airnodeAddress, stage, airnodeVersion, timestamp].join('')) | ||
| .digest('hex') | ||
| .substring(0, 8); |
There was a problem hiding this comment.
Maybe have a constant for the digest length?
| @@ -354,7 +349,7 @@ describe('terraformAirnodeApply', () => { | |||
| ); | |||
| expect(exec).toHaveBeenNthCalledWith( | |||
| 2, | |||
| `terraform apply -var="aws_region=us-east-1" -var="airnode_address_short=a30ca71" -var="stage=dev" -var="configuration_file=${configPath}" -var="secrets_file=${secretsPath}" -var="handler_dir=${handlerDir}" -var="disable_concurrency_reservation=false" -var="airnode_wallet_private_key=0xd627c727db73ed7067cbc1e15295f7004b83c01d243aa90711d549cda6bd5bca" -input=false -no-color -var="max_concurrency=100" -var="http_gateway_enabled=true" -var="http_max_concurrency=20" -var="http_signed_data_gateway_enabled=true" -var="http_signed_data_max_concurrency=20" -auto-approve`, | |||
| `terraform apply -var="aws_region=us-east-1" -var="deployment_id=aws40207f25" -var="configuration_file=${configPath}" -var="secrets_file=${secretsPath}" -var="handler_dir=${handlerDir}" -var="disable_concurrency_reservation=false" -var="airnode_wallet_private_key=0xd627c727db73ed7067cbc1e15295f7004b83c01d243aa90711d549cda6bd5bca" -input=false -no-color -var="max_concurrency=100" -var="http_gateway_enabled=true" -var="http_max_concurrency=20" -var="http_signed_data_gateway_enabled=true" -var="http_signed_data_max_concurrency=20" -auto-approve`, | |||
There was a problem hiding this comment.
I know I was a bit sad when I had to edit this line, but I didn't want to change it at that time... WDYT if we split it by the parameters. E.g.:
[`terraform apply`, `-var="aws_region=us-east-1"`, `-var="deployment_id=aws40207f25"`, `-var="configuration_file=${configPath}"`...].join(' ')
| name_prefix = "${var.infrastructure_name}-${var.airnode_address_short}-${var.stage}" | ||
| # deployment_id - 11 characters | ||
| # dash between - 1 character | ||
| name_prefix = "${var.infrastructure_name}-${var.deployment_id}" |
There was a problem hiding this comment.
Shouldn't we make the deployment ID a bit longer? The 8 characters length seems to me unreasoanbly low.
There was a problem hiding this comment.
Why? Are you worried about collisions?
There was a problem hiding this comment.
Yeah, and I do not see a reason why not make it longer when the character limit allows it.
There was a problem hiding this comment.
One reason would be so that the outputs of list and info deployer commands won't be that wide. But I'll take a look. Maybe 12 or even 16 would be doable.
There was a problem hiding this comment.
I'll keep the 8-character deployment IDs for now. The rows from the list and info commands are around 120 characters long now and I don't want to increase that more. Also, the probability of a collision is still quite low. From a quick googling the problem is not with the hash function creating a collision but rather running into a collision purely based on limited possibilities from the pool of strings of that length and composition (https://stackoverflow.com/a/30565118). But even that, gives us about 65k possible hashes for 8-character long substrings (https://www.bluebill.net/hash_collisions.html). No one account will have that.
amarthadan
force-pushed
the
remove-airnode-short-address
branch
from
042a128
to
6951537
Compare
Close #1486