From 7de9725dd17eb3d432c4d4070e348fb665557125 Mon Sep 17 00:00:00 2001 From: Richard Maynard <112959480+rmaynardap@users.noreply.github.com> Date: Fri, 25 Aug 2023 10:24:04 -0500 Subject: [PATCH] INF-5144: validate planfile lineage (#31) This PR ensures that a provided backend plan matches the current terraform state. This is an important check to make sure that the plan file being consumed is appropriate for the terraform state. In addition the following minor changes are also included: * only load backend handlers when --backend-plans is provided * remove extraneous DEBUG message --- tfworker/backends/s3.py | 57 +++++++++++++++++++++++++++++----- tfworker/commands/base.py | 2 +- tfworker/commands/terraform.py | 3 -- 3 files changed, 51 insertions(+), 11 deletions(-) diff --git a/tfworker/backends/s3.py b/tfworker/backends/s3.py index ae182dd..c7f33bb 100644 --- a/tfworker/backends/s3.py +++ b/tfworker/backends/s3.py @@ -18,6 +18,7 @@ from contextlib import closing from pathlib import Path from uuid import uuid4 +from zipfile import ZipFile import boto3 import botocore @@ -391,6 +392,7 @@ def _check_plan(self, planfile: Path, definition: str, **kwargs): """check_plan runs while the plan is being checked, it should fetch a file from the backend and store it in the local location""" # ensure planfile does not exist or is zero bytes if it does remotefile = f"{self._authenticator.prefix}/{definition}/{planfile.name}" + statefile = f"{self._authenticator.prefix}/{definition}/terraform.tfstate" if planfile.exists(): if planfile.stat().st_size == 0: planfile.unlink() @@ -400,10 +402,20 @@ def _check_plan(self, planfile: Path, definition: str, **kwargs): if self._s3_get_plan(planfile, remotefile): if not planfile.exists(): raise HandlerError(f"planfile not found after download: {planfile}") - click.secho( - f"remote planfile downloaded: s3://{self._authenticator.bucket}/{remotefile} -> {planfile}", - fg="yellow", - ) + # verify the lineage and serial from the planfile matches the statefile + if not self._verify_lineage(planfile, statefile): + click.secho( + f"planfile lineage does not match statefile, remote plan is unsuitable and will be removed", + fg="red", + ) + self._s3_delete_plan(remotefile) + planfile.unlink() + else: + click.secho( + f"remote planfile downloaded: s3://{self._authenticator.bucket}/{remotefile} -> {planfile}", + fg="yellow", + ) + return None def _post_plan( self, planfile: Path, definition: str, changes: bool = False, **kwargs @@ -433,12 +445,12 @@ def _pre_apply(self, planfile: Path, definition: str, **kwargs): logfile = planfile.with_suffix(".log") remotefile = f"{self._authenticator.prefix}/{definition}/{planfile.name}" remotelog = remotefile.replace(".tfplan", ".log") - if self._s3_delete_plan(remotefile, planfile): + if self._s3_delete_plan(remotefile): click.secho( f"remote planfile removed: s3://{self._authenticator.bucket}/{remotefile}", fg="yellow", ) - if self._s3_delete_plan(remotelog, logfile): + if self._s3_delete_plan(remotelog): click.secho( f"remote logfile removed: s3://{self._authenticator.bucket}/{remotelog}", fg="yellow", @@ -478,7 +490,7 @@ def _s3_put_plan(self, planfile: Path, remotefile: str) -> bool: raise HandlerError(f"Error uploading planfile: {e}") return uploaded - def _s3_delete_plan(self, remotefile: str, planfile: str) -> bool: + def _s3_delete_plan(self, remotefile: str) -> bool: """_delete_plan removes a remote plan file""" deleted = False try: @@ -489,3 +501,34 @@ def _s3_delete_plan(self, remotefile: str, planfile: str) -> bool: except botocore.exceptions.ClientError as e: raise HandlerError(f"Error deleting planfile: {e}") return deleted + + def _verify_lineage(self, planfile: Path, statefile: str) -> bool: + # load the statefile as a json object from the backend + state = None + try: + state = json.loads( + self._s3_client.get_object( + Bucket=self._authenticator.bucket, Key=statefile + )["Body"].read() + ) + except botocore.exceptions.ClientError as e: + raise HandlerError(f"Error downloading statefile: {e}") + + # load the planfile as a json object + plan = None + try: + with ZipFile(str(planfile), "r") as zip: + with zip.open("tfstate") as f: + plan = json.loads(f.read()) + except Exception as e: + raise HandlerError(f"Error loading planfile: {e}") + + # compare the lineage and serial from the planfile to the statefile + if not (state and plan): + return False + if state["serial"] != plan["serial"]: + return False + if state["lineage"] != plan["lineage"]: + return False + + return True diff --git a/tfworker/commands/base.py b/tfworker/commands/base.py index 0117ea2..a30b6bc 100644 --- a/tfworker/commands/base.py +++ b/tfworker/commands/base.py @@ -135,7 +135,7 @@ def __init__(self, rootc, deployment="undefined", limit=tuple(), **kwargs): raise SystemExit(1) # allow a backend to implement handlers as well since they already control the provider session - if self._backend.handlers: + if self._backend.handlers and self._backend_plans: self._handlers.update(self._backend.handlers) @property diff --git a/tfworker/commands/terraform.py b/tfworker/commands/terraform.py index 99abc45..ebbd222 100644 --- a/tfworker/commands/terraform.py +++ b/tfworker/commands/terraform.py @@ -283,9 +283,6 @@ def _check_apply_or_destroy(self, changes, definition) -> bool: def _exec_apply_or_destroy(self, definition) -> None: """_exec_apply_or_destroy executes a terraform apply or destroy""" # call handlers for pre apply - click.secho( - f"DEBUG: executing {self._plan_for} for {definition.tag}", fg="yellow" - ) try: self._execute_handlers( action=self._plan_for,