fix bug with checksums causing planfile not to be written (cloudposse#13)

Co-authored-by: mcalhoun <mcalhoun@users.noreply.github.com>

Author: Matt Calhoun

.github/workflows/build-and-test.yml

@@ -61,6 +61,13 @@ jobs:
           aws-region: ${{ env.AWS_REGION }}
           role-session-name: "atmos-terraform-plan-gitops"
 
+      - name: Calculate SHA256 of planfile before storage
+        id: before-store-checksum
+        run: |
+          checksum=$(shasum -a 256 src/__fixtures__/mock.planfile | cut -d ' ' -f 1)
+          echo "$checksum"
+          echo "checksum=$checksum" >>$GITHUB_OUTPUT
+
       - uses: ./
         id: store-plan
         with:
@@ -84,3 +91,17 @@ jobs:
           bucketName: ${{ env.ATMOS_PLAN_S3_BUCKET }}
         env:
           GITHUB_TOKEN: ${{ secrets.PUBLIC_REPO_ACCESS_TOKEN }}
+
+      - name: Calculate SHA256 of planfile after retrieval
+        id: after-get-checksum
+        run: |
+          checksum=$(shasum -a 256 ./test.planfile | cut -d ' ' -f 1)
+          echo "$checksum"
+          echo "checksum=$checksum" >>$GITHUB_OUTPUT
+
+      - name: Compare Before and After Checksums
+        run: |
+          if [ "${{ steps.before-store-checksum.outputs.checksum }}" != "${{ steps.after-get-checksum.outputs.checksum }}" ]; then
+            echo "Checksums do not match"
+            exit 1
+          fi

dist/index.js

@@ -60421,12 +60421,16 @@ exports.taintPlan = taintPlan;
 "use strict";
 
 Object.defineProperty(exports, "__esModule", ({ value: true }));
-exports.calculateHash = void 0;
+exports.calculateHashFromBuffer = exports.calculateHash = void 0;
 const node_crypto_1 = __nccwpck_require__(6005);
 const calculateHash = (plan) => {
     return (0, node_crypto_1.createHash)("sha256").update(plan).digest("hex");
 };
 exports.calculateHash = calculateHash;
+const calculateHashFromBuffer = (plan) => {
+    return (0, node_crypto_1.createHash)("sha256").update(plan).digest("hex");
+};
+exports.calculateHashFromBuffer = calculateHashFromBuffer;
 
 
 /***/ }),
@@ -61113,7 +61117,32 @@ __exportStar(__nccwpck_require__(51066), exports);
 
 /***/ }),
 
-/***/ 90320:
+/***/ 24748:
+/***/ (function(__unused_webpack_module, exports, __nccwpck_require__) {
+
+"use strict";
+
+var __createBinding = (this && this.__createBinding) || (Object.create ? (function(o, m, k, k2) {
+    if (k2 === undefined) k2 = k;
+    var desc = Object.getOwnPropertyDescriptor(m, k);
+    if (!desc || ("get" in desc ? !m.__esModule : desc.writable || desc.configurable)) {
+      desc = { enumerable: true, get: function() { return m[k]; } };
+    }
+    Object.defineProperty(o, k2, desc);
+}) : (function(o, m, k, k2) {
+    if (k2 === undefined) k2 = k;
+    o[k2] = m[k];
+}));
+var __exportStar = (this && this.__exportStar) || function(m, exports) {
+    for (var p in m) if (p !== "default" && !Object.prototype.hasOwnProperty.call(exports, p)) __createBinding(exports, m, p);
+};
+Object.defineProperty(exports, "__esModule", ({ value: true }));
+__exportStar(__nccwpck_require__(86325), exports);
+
+
+/***/ }),
+
+/***/ 86325:
 /***/ (function(__unused_webpack_module, exports) {
 
 "use strict";
@@ -61135,16 +61164,16 @@ var __asyncValues = (this && this.__asyncValues) || function (o) {
     function settle(resolve, reject, d, v) { Promise.resolve(v).then(function(v) { resolve({ value: v, done: d }); }, reject); }
 };
 Object.defineProperty(exports, "__esModule", ({ value: true }));
-exports.bufferFromReadable = void 0;
-const bufferFromReadable = (readable) => { var _a, readable_1, readable_1_1; return __awaiter(void 0, void 0, void 0, function* () {
+exports.stringFromReadable = void 0;
+const stringFromReadable = (readable) => { var _a, readable_1, readable_1_1; return __awaiter(void 0, void 0, void 0, function* () {
     var _b, e_1, _c, _d;
-    const buffers = [];
+    const chunks = [];
     try {
         for (_a = true, readable_1 = __asyncValues(readable); readable_1_1 = yield readable_1.next(), _b = readable_1_1.done, !_b; _a = true) {
             _d = readable_1_1.value;
             _a = false;
-            const data = _d;
-            buffers.push(data);
+            const chunk = _d;
+            chunks.push(Buffer.from(chunk));
         }
     }
     catch (e_1_1) { e_1 = { error: e_1_1 }; }
@@ -61154,35 +61183,9 @@ const bufferFromReadable = (readable) => { var _a, readable_1, readable_1_1; ret
         }
         finally { if (e_1) throw e_1.error; }
     }
-    const finalBuffer = Buffer.concat(buffers);
-    return finalBuffer;
+    return Buffer.concat(chunks).toString("utf-8");
 }); };
-exports.bufferFromReadable = bufferFromReadable;
-
-
-/***/ }),
-
-/***/ 24748:
-/***/ (function(__unused_webpack_module, exports, __nccwpck_require__) {
-
-"use strict";
-
-var __createBinding = (this && this.__createBinding) || (Object.create ? (function(o, m, k, k2) {
-    if (k2 === undefined) k2 = k;
-    var desc = Object.getOwnPropertyDescriptor(m, k);
-    if (!desc || ("get" in desc ? !m.__esModule : desc.writable || desc.configurable)) {
-      desc = { enumerable: true, get: function() { return m[k]; } };
-    }
-    Object.defineProperty(o, k2, desc);
-}) : (function(o, m, k, k2) {
-    if (k2 === undefined) k2 = k;
-    o[k2] = m[k];
-}));
-var __exportStar = (this && this.__exportStar) || function(m, exports) {
-    for (var p in m) if (p !== "default" && !Object.prototype.hasOwnProperty.call(exports, p)) __createBinding(exports, m, p);
-};
-Object.defineProperty(exports, "__esModule", ({ value: true }));
-__exportStar(__nccwpck_require__(90320), exports);
+exports.stringFromReadable = stringFromReadable;
 
 
 /***/ }),
@@ -61394,7 +61397,7 @@ const readFile = (path) => {
 exports.readFile = readFile;
 const writeFile = (path, contents) => {
     const outputStream = (0, fs_1.createWriteStream)(path);
-    contents.pipe(outputStream);
+    contents.pipe(outputStream, { end: true });
 };
 exports.writeFile = writeFile;
 
@@ -61519,7 +61522,7 @@ class TerraformPlan extends domain_1.AggregateRoot {
         if (guardResult.isFailure) {
             return infrastructure_1.Result.fail(guardResult.getErrorValue());
         }
-        const defaultValues = Object.assign(Object.assign({}, props), { contentsHash: (0, crypto_1.calculateHash)(props.contents), createdAt: props.createdAt ? props.createdAt : new Date(), tainted: props.tainted ? props.tainted : false });
+        const defaultValues = Object.assign(Object.assign({}, props), { contentsHash: props.contentsHash || (0, crypto_1.calculateHashFromBuffer)(props.contents), createdAt: props.createdAt ? props.createdAt : new Date(), tainted: props.tainted ? props.tainted : false });
         const terraformPlan = new TerraformPlan(defaultValues, id);
         return infrastructure_1.Result.ok(terraformPlan);
     }
@@ -61866,6 +61869,7 @@ class TerraformPlanDynamoDBMapper extends mapper_1.Mapper {
             tainted: raw.tainted,
             createdAt: new Date(raw.createdAt),
             contents: Buffer.from(""),
+            contentsHash: raw.contentsHash,
         }, new lib_1.UniqueEntityId(raw.id));
         if (planOrError.isFailure) {
             throw new Error("Error converting DynamoDB item to domain");
@@ -62294,6 +62298,7 @@ var __awaiter = (this && this.__awaiter) || function (thisArg, _arguments, P, ge
 Object.defineProperty(exports, "__esModule", ({ value: true }));
 exports.GetTerraformPlanUseCase = void 0;
 const fs_1 = __nccwpck_require__(57147);
+const stream_1 = __nccwpck_require__(12781);
 const crypto_1 = __nccwpck_require__(65788);
 const infrastructure_1 = __nccwpck_require__(73950);
 const readable_1 = __nccwpck_require__(24748);
@@ -62305,6 +62310,7 @@ const writePlanFile = (pathToPlan, contents) => __awaiter(void 0, void 0, void 0
         return (0, infrastructure_1.left)(new errors_1.GetTerraformPlanErrors.PlanAlreadyExistsError(pathToPlan));
     }
     (0, system_1.writeFile)(pathToPlan, contents);
+    return (0, infrastructure_1.right)(infrastructure_1.Result.ok());
 });
 class GetTerraformPlanUseCase {
     constructor(metaDataRepository, planRepository, codeRepository) {
@@ -62313,31 +62319,39 @@ class GetTerraformPlanUseCase {
         this.codeRepository = codeRepository;
     }
     execute(req) {
+        var _a, _b;
         return __awaiter(this, void 0, void 0, function* () {
             try {
                 const { commitSHA, component, isMergeCommit, stack, planPath, pr, repoName, repoOwner, } = req;
                 let plan;
+                let metadata;
                 if (isMergeCommit) {
                     if (!pr) {
                         return (0, infrastructure_1.left)(new infrastructure_1.AppError.UnexpectedError("PR is required for merge commits"));
                     }
-                    const metadata = yield this.metaDataRepository.loadLatestForPR(repoOwner, repoName, component, stack, pr);
+                    metadata = yield this.metaDataRepository.loadLatestForPR(repoOwner, repoName, component, stack, pr);
                     plan = yield this.planRepository.load(repoOwner, repoName, component, stack, metadata.commitSHA);
                 }
                 else {
                     // Non-merge commit, we're on the feature branch
                     if (!commitSHA) {
                         return (0, infrastructure_1.left)(new infrastructure_1.AppError.UnexpectedError("Commit is required for non-merge commits"));
                     }
-                    const metadata = yield this.metaDataRepository.loadByCommit(repoOwner, repoName, component, stack, commitSHA);
+                    metadata = yield this.metaDataRepository.loadByCommit(repoOwner, repoName, component, stack, commitSHA);
                     plan = yield this.planRepository.load(repoOwner, repoName, component, stack, metadata.commitSHA);
-                    const planBuffer = yield (0, readable_1.bufferFromReadable)(plan);
-                    const hash = yield (0, crypto_1.calculateHash)(planBuffer);
-                    if (metadata.contentsHash === hash) {
-                        return (0, infrastructure_1.left)(new errors_1.GetTerraformPlanErrors.ContentsHashMismatch(metadata.contentsHash, hash));
-                    }
                 }
-                yield writePlanFile(planPath, plan);
+                const contents = yield (0, readable_1.stringFromReadable)(plan);
+                const hash = yield (0, crypto_1.calculateHash)(contents);
+                if (metadata.contentsHash != hash) {
+                    return (0, infrastructure_1.left)(new errors_1.GetTerraformPlanErrors.ContentsHashMismatch((_b = (_a = metadata.contentsHash) === null || _a === void 0 ? void 0 : _a.toString()) !== null && _b !== void 0 ? _b : "", hash));
+                }
+                const contentsReadable = new stream_1.Readable();
+                contentsReadable.push(contents);
+                contentsReadable.push(null);
+                const result = yield writePlanFile(planPath, contentsReadable);
+                if (result.isLeft()) {
+                    return (0, infrastructure_1.left)(result.value);
+                }
                 return (0, infrastructure_1.right)(infrastructure_1.Result.ok());
             }
             catch (err) {

dist/index.js.map

@@ -1,9 +1,18 @@
-import { calculateHash } from "./crypto";
+import { calculateHash, calculateHashFromBuffer } from "./crypto";
 
 describe("crypto", () => {
-  describe("createHash", () => {
+  describe("calculateHash", () => {
     it("should calculate hash", () => {
-      const hash = calculateHash(Buffer.from("hello"));
+      const hash = calculateHash("hello");
+      expect(hash).toEqual(
+        "2cf24dba5fb0a30e26e83b2ac5b9e29e1b161e5c1fa7425e73043362938b9824"
+      );
+    });
+  });
+
+  describe("calculateHashFromBuffer", () => {
+    it("should calculate hash", () => {
+      const hash = calculateHashFromBuffer(Buffer.from("hello"));
       expect(hash).toEqual(
         "2cf24dba5fb0a30e26e83b2ac5b9e29e1b161e5c1fa7425e73043362938b9824"
       );

src/lib/crypto/crypto.test.ts

@@ -1,5 +1,9 @@
 import { createHash } from "node:crypto";
 
-export const calculateHash = (plan: Buffer) => {
+export const calculateHash = (plan: string) => {
+  return createHash("sha256").update(plan).digest("hex");
+};
+
+export const calculateHashFromBuffer = (plan: Buffer) => {
   return createHash("sha256").update(plan).digest("hex");
 };

src/lib/crypto/crypto.ts

@@ -1 +1 @@
-export * from "./bufferFromReadable";
+export * from "./stringFromReadable";

src/lib/readable/bufferFromReadable.ts

@@ -0,0 +1,13 @@
+import { Readable } from "stream";
+
+export const stringFromReadable = async (
+  readable: Readable
+): Promise<string> => {
+  const chunks = [];
+
+  for await (const chunk of readable) {
+    chunks.push(Buffer.from(chunk));
+  }
+
+  return Buffer.concat(chunks).toString("utf-8");
+};

src/lib/readable/index.ts

@@ -7,5 +7,5 @@ export const readFile = (path: string): Buffer => {
 
 export const writeFile = (path: string, contents: Readable): void => {
   const outputStream = createWriteStream(path);
-  contents.pipe(outputStream);
+  contents.pipe(outputStream, { end: true });
 };

src/lib/readable/stringFromReadable.ts

@@ -1,4 +1,4 @@
-import { calculateHash } from "@lib/crypto";
+import { calculateHash, calculateHashFromBuffer } from "@lib/crypto";
 import { AggregateRoot, UniqueEntityId } from "@lib/domain";
 import { Guard, IGuardArgument, Result } from "@lib/infrastructure";
 
@@ -69,7 +69,8 @@ export class TerraformPlan extends AggregateRoot<TerraformPlanProps> {
 
     const defaultValues: TerraformPlanProps = {
       ...props,
-      contentsHash: calculateHash(props.contents),
+      contentsHash:
+        props.contentsHash || calculateHashFromBuffer(props.contents),
       createdAt: props.createdAt ? props.createdAt : new Date(),
       tainted: props.tainted ? props.tainted : false,
     };