diff --git a/.github/workflows/deploy.yml b/.github/workflows/deploy.yml index 70c5166..5071640 100644 --- a/.github/workflows/deploy.yml +++ b/.github/workflows/deploy.yml @@ -35,6 +35,35 @@ jobs: runs-on: ubuntu-latest timeout-minutes: 30 steps: + # An unset secret expands to an empty string rather than failing, which + # otherwise surfaces much later as an opaque CloudFormation + # AWS::EarlyValidation::PropertyValidation error. Catch it here instead, + # naming the secrets that are actually missing. + - name: Check required secrets are present + env: + AWS_ROLE_ARN: ${{ secrets.AWS_ROLE_ARN }} + RDS_HOSTNAME: ${{ secrets.RDS_HOSTNAME }} + RDS_USERNAME: ${{ secrets.RDS_USERNAME }} + RDS_PASSWORD: ${{ secrets.RDS_PASSWORD }} + DCITE: ${{ secrets.DCITE }} + VPC_ID: ${{ secrets.VPC_ID }} + PRIVATE_SUBNETS: ${{ secrets.PRIVATE_SUBNETS }} + RDS_SECURITY_GROUP_ID: ${{ secrets.RDS_SECURITY_GROUP_ID }} + ALERT_EMAIL: ${{ secrets.ALERT_EMAIL }} + run: | + missing=() + for v in AWS_ROLE_ARN RDS_HOSTNAME RDS_USERNAME RDS_PASSWORD DCITE \ + VPC_ID PRIVATE_SUBNETS RDS_SECURITY_GROUP_ID ALERT_EMAIL; do + [ -n "${!v:-}" ] || missing+=("$v") + done + if [ ${#missing[@]} -gt 0 ]; then + echo "::error::Missing or empty secrets: ${missing[*]}" + echo "Repository secrets live in Settings -> Secrets and variables -> Actions." + echo "Organisation secrets must additionally grant this repository access." + exit 1 + fi + echo "All 9 required secrets are present." + - name: Checkout code uses: actions/checkout@v5 diff --git a/.github/workflows/run-minting.yml b/.github/workflows/run-minting.yml index afc82ec..8752ace 100644 --- a/.github/workflows/run-minting.yml +++ b/.github/workflows/run-minting.yml @@ -55,6 +55,7 @@ jobs: echo "CLUSTER=$(get_output ClusterName)" >> $GITHUB_ENV echo "TASK_DEF=$(get_output TaskDefinitionArn)" >> $GITHUB_ENV echo "TASK_SG=$(get_output TaskSecurityGroupId)" >> $GITHUB_ENV + echo "TASK_SUBNETS=$(get_output PrivateSubnetIds)" >> $GITHUB_ENV echo "LOG_BUCKET=$(get_output LogBucketName)" >> $GITHUB_ENV echo "LOG_GROUP=$(get_output LogGroupName)" >> $GITHUB_ENV @@ -78,7 +79,9 @@ jobs: + (if ($cmd | length) > 0 then {command: $cmd} else {} end)) ]}') - NETWORK=$(jq -nc --arg sg "$TASK_SG" --arg subnets "${{ secrets.PRIVATE_SUBNETS }}" ' + # Subnets come from the stack output rather than a secret, so there is + # one fewer value that can silently be empty at run time. + NETWORK=$(jq -nc --arg sg "$TASK_SG" --arg subnets "$TASK_SUBNETS" ' {awsvpcConfiguration: { subnets: ($subnets | split(",")), securityGroups: [$sg], diff --git a/infrastructure/doi-minter.yaml b/infrastructure/doi-minter.yaml index 1ffb4ef..cff594e 100644 --- a/infrastructure/doi-minter.yaml +++ b/infrastructure/doi-minter.yaml @@ -319,15 +319,10 @@ Resources: taskArn: $.detail.taskArn stoppedAt: $.detail.stoppedAt stoppedReason: $.detail.stoppedReason - InputTemplate: !Sub | - "The Neotoma DOI minting run (${Environment}) failed." - "" - "Task: " - "Stopped: " - "Reason: " - "" - "Container logs: /ecs/neotoma-doi-minter-${Environment} in CloudWatch." - "Run logs: s3://neotoma-doi-logs-${Environment}-${AWS::AccountId}/" + # A single quoted string: EventBridge emits one quoted line per + # template line, so a multi-line template reaches SNS with literal + # quote characters around every line. + InputTemplate: !Sub '"The Neotoma DOI minting run (${Environment}) failed. Task stopped at . Reason: . Container logs: log group /ecs/neotoma-doi-minter-${Environment} in CloudWatch. Run logs: s3://neotoma-doi-logs-${Environment}-${AWS::AccountId}/"' # The task never got as far as running its container (image pull failure, # missing secret, no capacity). This produces no exit code, so it needs its @@ -351,11 +346,7 @@ Resources: InputPathsMap: taskArn: $.detail.taskArn stoppedReason: $.detail.stoppedReason - InputTemplate: !Sub | - "The Neotoma DOI minting task (${Environment}) failed to start." - "" - "Task: " - "Reason: " + InputTemplate: !Sub '"The Neotoma DOI minting task (${Environment}) failed to start. Task . Reason: "' Outputs: ClusterName: @@ -376,6 +367,15 @@ Outputs: Export: Name: !Sub "${AWS::StackName}-TaskSecurityGroupId" + PrivateSubnetIds: + Description: > + Subnets the task runs in. Exported so manual `run-task` invocations can + read them from the stack instead of needing their own copy of the + PRIVATE_SUBNETS secret. + Value: !Join [",", !Ref PrivateSubnets] + Export: + Name: !Sub "${AWS::StackName}-PrivateSubnetIds" + LogBucketName: Description: Bucket holding the per-run minting logs. Value: !Ref LogBucket diff --git a/ndbdoi.py b/ndbdoi.py index 7e95c5f..02d387d 100644 --- a/ndbdoi.py +++ b/ndbdoi.py @@ -113,6 +113,7 @@ def main(args): _ = printargs(args, datasetids) errors = 0 + skipped = 0 for dataset_id in datasetids: try: # Create and configure DOI object @@ -147,6 +148,13 @@ def main(args): else: print(f"○ Dataset {dataset_id}: Skipped (already has DOI: {doi_obj.identifiers.get('identifier')})") + except neotomadoi.DatasetNotReady as e: + # Not a fault: the dataset simply is not ready to be minted yet. + # Log it and carry on so one such record cannot block the rest of + # the run. No errored.log entry, and no effect on the exit code. + skipped += 1 + print(f"○ Dataset {dataset_id}: Skipped - {str(e)}") + except Exception as e: errors += 1 print(f"✗ Dataset {dataset_id}: Failed - {str(e)}") @@ -159,7 +167,8 @@ def main(args): f.write("\n") print("-" * 50) - print(f"Processing complete: {len(datasetids) - errors} ok, {errors} failed") + print(f"Processing complete: {len(datasetids) - errors - skipped} ok, " + f"{skipped} skipped, {errors} failed") return 1 if errors else 0 if __name__ == '__main__': diff --git a/src/neotomadoi/__init__.py b/src/neotomadoi/__init__.py index 715c5dc..1b9dbe5 100644 --- a/src/neotomadoi/__init__.py +++ b/src/neotomadoi/__init__.py @@ -1,6 +1,7 @@ from .credentials import credentials from .databaseMode import databaseMode from .dataciteTestMode import dataciteTestMode +from .exceptions import DatasetNotReady from .fetch_metadata import ( neo_contributors, neo_creators, @@ -17,6 +18,7 @@ from .neotomaDOI import activity, neotomaDOI __all__ = [ + "DatasetNotReady", "dataciteTestMode", "databaseMode", "neo_connect", diff --git a/src/neotomadoi/exceptions.py b/src/neotomadoi/exceptions.py new file mode 100644 index 0000000..27d2266 --- /dev/null +++ b/src/neotomadoi/exceptions.py @@ -0,0 +1,9 @@ +class DatasetNotReady(Exception): + """_The dataset cannot be minted yet, through no fault of the system._ + + Raised for conditions that are expected to resolve on their own — most + commonly a dataset with no submission date, meaning its owner has not + submitted it yet. Callers should skip these datasets and carry on rather + than treating them as failures, so that one not-yet-ready record does not + block minting for everything else in the same run. + """ diff --git a/src/neotomadoi/neotomaDOI.py b/src/neotomadoi/neotomaDOI.py index dc10b10..5671748 100644 --- a/src/neotomadoi/neotomaDOI.py +++ b/src/neotomadoi/neotomaDOI.py @@ -15,6 +15,7 @@ from .credentials import credentials from .databaseMode import databaseMode from .dataciteTestMode import dataciteTestMode +from .exceptions import DatasetNotReady from .fetch_metadata import ( neo_contributors, neo_creators, @@ -521,14 +522,22 @@ def mint_doi(self, force: bool = False): _ = self.validate() payload = {"type": "dois", "attributes": self.data} - date = min( - [ - datetime.strptime(i.get("date"), "%Y-%m-%d") - for i in self.data.get("dates") - if i.get("dateType") == "Submitted" - ] - ) + submitted = [ + datetime.strptime(i.get("date"), "%Y-%m-%d") + for i in self.data.get("dates") + if i.get("dateType") == "Submitted" + ] + # No submission date means the owner has not submitted the dataset yet, + # which is a normal transient state rather than a fault. Raise in both + # modes, not just `prod`: the sandbox pass is a rehearsal of the mint, so + # it should skip exactly what production would skip. + if not submitted: + raise DatasetNotReady( + f"dataset {self.datasetid} has no submission date; not submitted yet" + ) + if self.dataciteMode.name == "prod": + date = min(submitted) if datetime.now() - date > timedelta(days=2): payload["attributes"]["event"] = "publish" elif force: