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

fix(dockerfile): add dig to image #1244

Merged
merged 1 commit into from
Mar 27, 2024
Merged

fix(dockerfile): add dig to image #1244

merged 1 commit into from
Mar 27, 2024

Conversation

seaerchin
Copy link
Contributor

@seaerchin seaerchin requested a review from a team March 27, 2024 14:36
@seaerchin
Copy link
Contributor Author

flakey timestamp test failed - not the first time, maybe we should relax the test

@kishore03109
Copy link
Contributor

lol i think we shouldnt use that library in the first place. can merge this for release tmr, but think we can just directly use node's dig

@seaerchin seaerchin merged commit 2b13015 into develop Mar 27, 2024
18 of 19 checks passed
@seaerchin seaerchin deleted the fix/add-dig branch March 27, 2024 15:31
@kishore03109 kishore03109 mentioned this pull request Mar 28, 2024
3 tasks
kishore03109 added a commit that referenced this pull request Mar 28, 2024
* chore(package): use npm (#1237)

* fix: upgrade dotenv from 16.4.1 to 16.4.5 (#1233)

Snyk has created this PR to upgrade dotenv from 16.4.1 to 16.4.5.

See this package in npm:
https://www.npmjs.com/package/dotenv

See this project in Snyk:
https://app.snyk.io/org/isomer/project/676b9e26-cebf-4964-b7b3-d9843e3339ff?utm_source=github&utm_medium=referral&page=upgrade-pr

Co-authored-by: snyk-bot <[email protected]>

* build(deps): bump express from 4.17.3 to 4.19.2 (#1242)

Bumps [express](https://github.com/expressjs/express) from 4.17.3 to 4.19.2.
- [Release notes](https://github.com/expressjs/express/releases)
- [Changelog](https://github.com/expressjs/express/blob/master/History.md)
- [Commits](expressjs/express@4.17.3...4.19.2)

---
updated-dependencies:
- dependency-name: express
  dependency-type: direct:production
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* fix: package.json & package-lock.json to reduce vulnerabilities (#1241)

The following vulnerabilities are fixed with an upgrade:
- https://snyk.io/vuln/SNYK-JS-EXPRESS-6474509

Co-authored-by: snyk-bot <[email protected]>

* fix: upgrade @growthbook/growthbook from 0.27.0 to 0.34.0 (#1232)

Snyk has created this PR to upgrade @growthbook/growthbook from 0.27.0 to 0.34.0.

See this package in npm:
https://www.npmjs.com/package/@growthbook/growthbook

See this project in Snyk:
https://app.snyk.io/org/isomer/project/676b9e26-cebf-4964-b7b3-d9843e3339ff?utm_source=github&utm_medium=referral&page=upgrade-pr

Co-authored-by: snyk-bot <[email protected]>

* fix: upgrade @aws-sdk/client-dynamodb from 3.501.0 to 3.521.0 (#1229)

Snyk has created this PR to upgrade @aws-sdk/client-dynamodb from 3.501.0 to 3.521.0.

See this package in npm:
https://www.npmjs.com/package/@aws-sdk/client-dynamodb

See this project in Snyk:
https://app.snyk.io/org/isomer/project/676b9e26-cebf-4964-b7b3-d9843e3339ff?utm_source=github&utm_medium=referral&page=upgrade-pr

Co-authored-by: snyk-bot <[email protected]>

* fix: upgrade @aws-sdk/client-amplify from 3.501.0 to 3.521.0 (#1230)

Snyk has created this PR to upgrade @aws-sdk/client-amplify from 3.501.0 to 3.521.0.

See this package in npm:
https://www.npmjs.com/package/@aws-sdk/client-amplify

See this project in Snyk:
https://app.snyk.io/org/isomer/project/676b9e26-cebf-4964-b7b3-d9843e3339ff?utm_source=github&utm_medium=referral&page=upgrade-pr

Co-authored-by: snyk-bot <[email protected]>

* fix: upgrade @aws-sdk/client-cloudwatch-logs from 3.501.0 to 3.521.0 (#1231)

Snyk has created this PR to upgrade @aws-sdk/client-cloudwatch-logs from 3.501.0 to 3.521.0.

See this package in npm:
https://www.npmjs.com/package/@aws-sdk/client-cloudwatch-logs

See this project in Snyk:
https://app.snyk.io/org/isomer/project/676b9e26-cebf-4964-b7b3-d9843e3339ff?utm_source=github&utm_medium=referral&page=upgrade-pr

Co-authored-by: snyk-bot <[email protected]>

* fix(dockerfile): add dig to image (#1244)

* feat(dd): add traces to gitfilesysteM (#1240)

* feat(dd): adding in metrics to gitfilesystem

* feat: instrument with wrap-tracers

* feat: patch dd-trace to understand neverthrow

* fix: missing tracer import

* fix: use the clear tracer.wrap() method

* fix(dd): fix patch type

* chore: add tests for tracer compatibility with neverthrow

* chore: upgrade dd-trace to v5 and patch for neverthrow

* chore: remove unecessary change from patch

* feat: implement a blacklist for methods that should not be instrumented

* chore: remove obsolete patch

* chore: lock dd-trace to 5.9.0 strictly so patch is always applied

---------

Co-authored-by: Timothee Groleau <[email protected]>

* fix: reduce log size to just last commit (#1243)

* fix: reduce log size to just last commit

* feat: defaults maxCount in getGitLog to 1

* chore(GitFileSystemService): add validation tests for getGitLog

* chore(dep): add flag (#1247)

* chore: bump version to v0.73.0

---------

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: seaerchin <[email protected]>
Co-authored-by: Isomer Admin <[email protected]>
Co-authored-by: snyk-bot <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Timothee Groleau <[email protected]>
Co-authored-by: Timothee Groleau <[email protected]>
@timotheeg timotheeg mentioned this pull request Apr 2, 2024
9 tasks
@kishore03109 kishore03109 mentioned this pull request Apr 2, 2024
4 tasks
kishore03109 added a commit that referenced this pull request Apr 3, 2024
## Problem

We do have some checks in the backend to check prior to any site launch that there are AAAA records/CAA records present. In this case, the dig commands to check the AAAA records [failed](https://ogp.datadoghq.com/logs?query=service%3Aisomer%20env%3Aproduction%20%22An%20error%20occurred%20while%20performing%20dig%20for%20domain%3A%22%20&cols=host%2Cservice&fromUser=true&index=%2A&messageDisplay=inline&refresh_mode=sliding&storage=hot&stream_sort=desc&viz=stream&from_ts=1711457521321&to_ts=1711543921321&live=true).

The reason for above is that we were using a library called `node-dig-dns`. This called the dig command [directly](https://github.com/StephanGeorg/node-dig-dns/blob/master/src/index.js#L78) at a system level. However, our docker container does not have the dig command out of the box. This resulted in the existence of AAAA records not being caught.


We are also codifying a check for CAA records and ensuring that if there exist at least one  caa record and it uses our redirection service, it should have letsencrypt as one of the caa record. 

To prevent accidental commits to live indirection repo during dev, also adding a check to only commit to the indirection repository iff it is in prod environment. 
 
## Solution

just use node's dns resolver directly. this way we dont have to depend on an external library's implementation of node and dont have to install unnecessary deps in the docker.

remove dep introduced in #1244 

**Breaking Changes**

<!-- Does this PR contain any backward incompatible changes? If so, what are they and should there be special considerations for release? -->

- [ ] Yes - this PR contains breaking changes
  - Details ...
- [x] No - this PR is backwards compatible with ALL of the following feature flags in this [doc](https://www.notion.so/opengov/Existing-feature-flags-518ad2cdc325420893a105e88c432be5)

**Features**:
<img width="1493" alt="Screenshot 2024-03-27 at 11 21 44 PM" src="https://github.com/isomerpages/isomercms-backend/assets/42832651/73edd437-492b-4bff-86c9-3392cb40fe49">


### Manual test (not to be copied over to deployment notes)

- [ ] add these lines of code at the end of server.js 
```
const formResponses = [
  {
    submissionId: "",
    requesterEmail: "[email protected]",
    repoName: "kishore-test-dev-emil",
    primaryDomain: "google.com",
    redirectionDomain: "www.google.com",
    agencyEmail: "[email protected]",
  },
]

formsgSiteLaunchRouter.handleSiteLaunchResults(formResponses, "test")
```  

- [ ] Assert that the email comes out to
alexanderleegs added a commit that referenced this pull request Apr 4, 2024
* fix(server): server should die if unable to connect to db (#1265)

## Problem

something in me wanted to check if we indeed exit if we fail to connect to db, and the answer is... no 


[node.js](https://nodejs.org/docs/latest-v18.x/api/process.html#processexitcode_1) states that 
> A number which will be the process exit code, when the process either exits gracefully, or is exited via process.exit() without specifying a code.

so it does not actually do the exiting, which leads to silent failures

* fix(dig): dig not working (#1246)

## Problem

We do have some checks in the backend to check prior to any site launch that there are AAAA records/CAA records present. In this case, the dig commands to check the AAAA records [failed](https://ogp.datadoghq.com/logs?query=service%3Aisomer%20env%3Aproduction%20%22An%20error%20occurred%20while%20performing%20dig%20for%20domain%3A%22%20&cols=host%2Cservice&fromUser=true&index=%2A&messageDisplay=inline&refresh_mode=sliding&storage=hot&stream_sort=desc&viz=stream&from_ts=1711457521321&to_ts=1711543921321&live=true).

The reason for above is that we were using a library called `node-dig-dns`. This called the dig command [directly](https://github.com/StephanGeorg/node-dig-dns/blob/master/src/index.js#L78) at a system level. However, our docker container does not have the dig command out of the box. This resulted in the existence of AAAA records not being caught.


We are also codifying a check for CAA records and ensuring that if there exist at least one  caa record and it uses our redirection service, it should have letsencrypt as one of the caa record. 

To prevent accidental commits to live indirection repo during dev, also adding a check to only commit to the indirection repository iff it is in prod environment. 
 
## Solution

just use node's dns resolver directly. this way we dont have to depend on an external library's implementation of node and dont have to install unnecessary deps in the docker.

remove dep introduced in #1244 

**Breaking Changes**

<!-- Does this PR contain any backward incompatible changes? If so, what are they and should there be special considerations for release? -->

- [ ] Yes - this PR contains breaking changes
  - Details ...
- [x] No - this PR is backwards compatible with ALL of the following feature flags in this [doc](https://www.notion.so/opengov/Existing-feature-flags-518ad2cdc325420893a105e88c432be5)

**Features**:
<img width="1493" alt="Screenshot 2024-03-27 at 11 21 44 PM" src="https://github.com/isomerpages/isomercms-backend/assets/42832651/73edd437-492b-4bff-86c9-3392cb40fe49">


### Manual test (not to be copied over to deployment notes)

- [ ] add these lines of code at the end of server.js 
```
const formResponses = [
  {
    submissionId: "",
    requesterEmail: "[email protected]",
    repoName: "kishore-test-dev-emil",
    primaryDomain: "google.com",
    redirectionDomain: "www.google.com",
    agencyEmail: "[email protected]",
  },
]

formsgSiteLaunchRouter.handleSiteLaunchResults(formResponses, "test")
```  

- [ ] Assert that the email comes out to

* fix: remove unecessary join and site retrieval (#1268)

* Improve APM spans (no more <anonymous>) (#1267)

* refactor: rename wrapper based on original function, only when original.name exists

* feat: utility to name all methods of an object

* feat: ensure all route handlers are named

* feat: drop the bound prefix in span names

* chore: remove unused eslint rule disabling

---------

Co-authored-by: Alexander Lee <[email protected]>

* fix: external links in top level nav (#1272)

* chore: bump version to v0.76.0

---------

Co-authored-by: Timothee Groleau <[email protected]>
Co-authored-by: Kishore <[email protected]>
This was referenced Jun 27, 2024
@dcshzj dcshzj mentioned this pull request Jun 27, 2024
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.

2 participants