Currently, the entire repo code is copied to the EC2 instance for the app.
https://github.com/bitovi/github-actions-deploy-docker-to-ec2/blob/main/operations/_scripts/generate/generate_app_repo.sh#L11

We should introduce a new property (default to empty) that specifies a directory path to push to ec2.

t2.small might not be suitable for everyone.

Variable is in the Terraform code, need to map it into the GHA.

It would be mighty helpful to be able to add additional tags to the deployed resources from the Github Action

Sometimes EFS Mounting and unmountig fails or get stuck. We should make the ansible scripts more resilient if possible

We run a shell script to create a bucket and make it exit 0 if there's an error message in the execution.

We need something to validate we can use that bucket, and as the shell script will run in every execution...

The same time is needed to check for it before or after creation, so I think the best approach would be to just check if we can access it afterwards.

Terraform is complaining that the ELB log policy is depreciated and needs to be updated;

Warning: Argument is deprecated
│ 
│   with aws_s3_bucket.lb_access_logs,
│   on elb.tf line 7, in resource "aws_s3_bucket" "lb_access_logs":
│    7:   policy = <<POLICY
│    8: {
│    9:   "Id": "Policy",
│   10:   "Version": "2012-10-17",
│   11:   "Statement": [
│   12:     {
│   13:       "Action": [
│   14:         "s3:PutObject"
│   15:       ],
│   16:       "Effect": "Allow",
│   17:       "Resource": "arn:aws:s3:::${var.lb_access_bucket_name}/*",
│   18:       "Principal": {
│   19:         "AWS": [
│   20:           "${data.aws_elb_service_account.main.arn}"
│   21:         ]
│   22:       }
│   23:     }
│   24:   ]
│   25: }
│   26: POLICY
│ 
│ Use the aws_s3_bucket_policy resource instead

Current naming set's a default of SG for deployment

Need to fix that to something unique

cat: /home/runner/work/_actions/bitovi/github-actions-deploy-docker-to-ec2/v0.4.1/operations/bo-out.env: No such file or directory
Error: Process completed with exit code 1.

https://github.com/bitovi/devops-training-ec2-gha-example/actions/runs/3759339931/jobs/6388785638#step:2:2672

As it is right now, if by any reason the name get's checked and fails, bucket name will be that error message.

Check should happen outside variable definition

It'd be nice to abstract this from the user with things like:

    aws_efs_only: true - would skip ansible and apply -target for just the EFS
    postgres_only: true
    etc
    ```

current .env file is managed via the calling repo's Repository secrets via a DOT_ENV secret.

It would be nice to allow configuration to point to a secret name in AWS Secrets Manager to pull the secret from instead

GITHUB_IDENTIFIER="$($GITHUB_ACTION_PATH/operations/_scripts/generate/generate_identifier.sh)"
GITHUB_IDENTIFIER=$(echo $GITHUB_IDENTIFIER | tr "_" "-")

should this be moved into the generate_identifier.sh script?
Probably create a ticket as I think this change would be a medium effort at least

We could remove completely the option for the instance to have a public IP exposed, or make it optional through a variable.

associate_public_ip_address = true

Terraform variable get's overwritten without a default in the action.yaml
As we need this to deploy the instance and it's overwritten with an empty value, it breaks Terraform deployment of the VM.

this file should not contain app specific info:
https://github.com/bitovi/github-actions-deploy-docker-to-ec2/blob/main/operations/deployment/terraform/api-dotenv.tmpl

everything should be removed except PORT

Regardless of modification, every terraform apply contains a tag change for the log ELB

Logs

  # aws_s3_bucket.lb_access_logs will be updated in-place
  ~ resource "aws_s3_bucket" "lb_access_logs" {
        id                          = "github-action-deploy-nfs-testing-logs"
        tags                        = {
            "Name" = "github-action-deploy-nfs-testing-logs"
        }
      ~ tags_all                    = {
          + "GitHubBranchName"          = (known after apply)
            # (8 unchanged elements hidden)
        }
        # (10 unchanged attributes hidden)

        # (2 unchanged blocks hidden)
    }

Plan: 0 to add, 1 to change, 0 to destro

If not passed, bucket name should be auto-generated.
Looks like we are not generating the names correctly.

Add vars.DOT_ENV as a source for environment variables.

We should be able to print out the DNS records for the deployed app in another step, so that the user can access it and view it more clearly.

We could execute commands setting up environment variables with Terraform and then run an after script to pass them to the GHA

If the exposed port is different than the application port running in the EC2 instance, incoming traffic will be blocked.
We should add the ELB exposed port in the security group for it to work.
We can create a new security group only for the load balancer or use the existing one.

We can
Create a Security Group for the Load Balancer ("LB-SG")
Create a Security Group for the EC2 instances ("App-SG")
In App-SG, permit inbound traffic on the desired port from LB-SG

We should keep in mind that we'll need access through SSH to the EC2 instance to run Ansible

Update the terraform to use a modular approach.

Example

.
├── bitops.after-deploy.d
│   ├── delete-tf-state-bucket.sh
│   └── output_inventory.sh
├── bitops.before-deploy.d
│   └── create-tf-state-bucket.sh
├── env-out.tf
├── inventory.tmpl
├── locals.tf
├── main.tf
├── modules
│   ├── 01_acm.tf.skip
│   ├── 01_data.tf
│   ├── 02_networking.tf
│   ├── 03_ec2.tf
│   ├── 04_elb.tf
│   ├── 05_route53.tf.skip
│   ├── 10_ansible.tf
│   └── variables.tf
├── provider.tf
├── terraform.tfvars
└── variables.tf

Spot this as a requirement while working on the https://github.com/bitovi/chaoss-config/pull/5.

Sometimes in order to run an app properly there is a need to execute a few scripts.
For instance, it could be some preparation steps like changing the sysctl settings on the host or setting the folder permissions on the host machine for shared data dirs.

With that, allowing to run some pre-hooks from the user's repository would be great.

If branch contains any upper case letter, AWS will fail creating the bucket.
Transform the string to all lowercase.

Would be good if we can ensure there aren't any other breaking characters.

https://github.com/bitovi/github-actions-deploy-docker-to-ec2/blob/main/action.yaml

Should perform the output steps like:
https://github.com/bitovi/github-actions-deploy-stackstorm/blob/main/action.yaml#L114-L125

Issue identified:
we are dynamically creating bucket based on the branch name and it is failing in cases where branch name has / like for example feat/sbox-rds-tf-state
Error:
Invalid bucket name "bitovi-devops-training-ec2-gha-example-feat/sbox-rds-tf-state": Bucket name must match the regex "^[a-zA-Z0-9.-_]{1,255}$" or be an ARN matching the regex "^arn:(aws).:(s3|s3-object-lambda):[a-z-0-9]:[0-9]{12}:accesspoint[/:][a-zA-Z0-9-.]{1,63}$|^arn:(aws).*:s3-outposts:[a-z-0-9]+:[0-9]{12}:outpost[/:][a-zA-Z0-9-]{1,63}[/:]accesspoint[/:][a-zA-Z0-9-]{1,63}$"

As a default, we could pair ports one to one. If the env defines a port, we should expose that one.

But, it would be good to have the option to override the exposed ports and health check of the ELB

Logs

│ Warning: Value for undeclared variable
│ 
│ The root module does not declare a variable named "sub_domain" but a value was found in file
│ "terraform.tfvars". If you meant to use this value, add a "variable" block to the configuration.
│ 
│ To silence these warnings, use TF_VAR_... environment variables to provide certain "global" settings
│ to all configurations in your organization. To reduce the verbosity of these warnings, use the
│ -compact-warnings option.

Need to find a way to copy files faster

Error: error deleting S3 Bucket (bitovi-devops-xxx-logs): BucketNotEmpty: The bucket you tried to delete is not empty

In order to purge it and delete it on deletion, we could add the following to the terraform code

force_destroy = true

s3 bucket name length limit is 63 chars.
if the generated resource identifier is, for example, 60 characters, then the tf state bucket will append -tf-state and will be longer than the limit.

Example:
bitovi-devops-holiday-2022-training-test-action-app-subdir
length is 58, so it does not get shortened because of this code: https://github.com/bitovi/github-actions-deploy-docker-to-ec2/blob/main/operations/_scripts/generate/shorten_identifier.sh#L11

the bucket would be called: bitovi-devops-holiday-2022-training-test-action-app-subdir-tf-state (67 characters) which is longer than the limit.

to fix: we could remove the appending of -tf-state here: https://github.com/bitovi/github-actions-deploy-docker-to-ec2/blob/main/operations/_scripts/generate/generate_tf_state_bucket.sh#L8
However, this would be a breaking change, so we'd need an upgrade guide like:

• if you upgrade with a repo that already has a state bucket (i.e. already has been deployed), you'll need to set aws_resource_identifier to be what it was previously

local.fqdn_provided will never be != ""

data "aws_acm_certificate" "issued" {
  count = var.no_cert == "true" ? 0 : ( var.create_root_cert != "true" ? ( var.create_sub_cert != "true" ? ( local.fqdn_provided != "" ? 1 : 0 ) : 0 ) : 0 )
  domain = var.domain_name
}

Nice to have

https://github.com/actions/toolkit/blob/master/docs/commands.md#problem-matchers

Print error messages using
echo "::error::Error message"
And group parts of the deployment

https://github.com/bitovi/github-actions-deploy-docker-to-ec2/blob/main/operations/_scripts/deploy/deploy.sh#L68

https://github.com/bitovi/bitops/releases/tag/v2.3.0

Suggested change

if [[ $TF_STATE_BUCKET_DESTROY = true ]] ; then
if [[ $TERRAFORM_DESTROY = true ]] && [[ $TF_STATE_BUCKET_DESTROY = true ]]; then 

Spot while using this GHA with https://github.com/bitovi/chaoss-config/pull/5.

There should be a way to control the default VM disk size (8GB by default), which could be small for running an app with backends, when there is no need to additionally configure EFS for it.

hashicorp/terraform-provider-aws#18311
The aws lb has a known and low priority issue regarding default_tags causing tf to plan a change.

Similar to #23

data.aws_elb_service_account.main: Read complete after 0s [id=127311923021]
╷
│ Error: "name" cannot be longer than 32 characters: "b4i-d4s-t6g-ec2-gha-e5e-t5g-gha-d32"
│ 
│   with aws_elb.vm[0],
│   on elb.tf line 81, in resource "aws_elb" "vm":
│   81:   name               = "${var.aws_resource_identifier_supershort}"
│ 
╵
2022-12-07 14:17:21,803 root         WARNING 

We must ensure the supershort string is below the 32 character limit.

Support data (RDS, elasticache, etc) via Terraform (toggles in the action for things like use_postgres and then output the postgres url along with the "vm successfully deployed" message

So I’m doing some testing locally on the ec2 instance size causing failure issue.
I’ve found that;
The instance is actually created fine, that’s not what’s being complained about.
The local files for whatever reason are the source of the issue.

Logs

localfile.tf-dotenv

│ Error: Provider produced inconsistent final plan
│ 
│ When expanding the plan for local_file.tf-dotenv to include new values learned so far during apply,
│ provider "registry.terraform.io/hashicorp/local" produced an invalid new value for .content: was
│ cty.StringVal("\n####\n#### EC2
│ values\n####\nAWS_INSTANCE_URL=ec2-35-182-249-21.ca-central-1.compute.amazonaws.com\n\n"), but now
│ cty.StringVal("\n####\n#### EC2
│ values\n####\nAWS_INSTANCE_URL=ec2-3-96-158-170.ca-central-1.compute.amazonaws.com\n\n").
│ 
│ This is a bug in the provider, which should be reported in the provider's own issue tracker.

localfile.ansible_inventory

│ Error: Provider produced inconsistent final plan
│ 
│ When expanding the plan for local_file.ansible_inventory to include new values learned so far during
│ apply, provider "registry.terraform.io/hashicorp/local" produced an invalid new value for .content: was
│ cty.StringVal("bitops_servers:\n hosts:\n   35.182.249.21\n vars:\n   ansible_ssh_user: ubuntu\n
│ ansible_ssh_private_key_file:
│ /Volumes/Home/Bitovi/GithubActions/github-actions-deploy-docker-to-ec2/operations/deployment/terraform/.ssh/bitops-ssh-key.pem\n
│ app_repo_name: github-actions-deploy-stackstorm-testing\n   app_install_root: /home/ubuntu\n   mount_efs:
│ false\n   efs_url: \n   resource_identifier: github-action-deploy-vpc-testing\n
│ application_mount_target: data\n   efs_mount_target: \n   data_mount_target: /data\n"), but now
│ cty.StringVal("bitops_servers:\n hosts:\n   3.96.158.170\n vars:\n   ansible_ssh_user: ubuntu\n
│ ansible_ssh_private_key_file:
│ /Volumes/Home/Bitovi/GithubActions/github-actions-deploy-docker-to-ec2/operations/deployment/terraform/.ssh/bitops-ssh-key.pem\n
│ app_repo_name: github-actions-deploy-stackstorm-testing\n   app_install_root: /home/ubuntu\n   mount_efs:
│ false\n   efs_url: \n   resource_identifier: github-action-deploy-vpc-testing\n
│ application_mount_target: data\n   efs_mount_target: \n   data_mount_target: /data\n").
│ 
│ This is a bug in the provider, which should be reported in the provider's own issue tracker.
╵
[Bitovi] [terraform] $ tf state show aws_instance.server

SOLUTION

To change the size of an instance, a double run will need to be performed.

If the application deployed is intended to run in the root domain, then we should be able to support this.

An easy option would be a variable to define if this should be deployed to a root domain.

in operations/_scripts/deploy/deploy.sh, set ANSIBLE_SKIP_DEPLOY to true if either of the following are true:

• ANSIBLE_SKIP_DEPLOY: ${{ inputs.infrastructure_only }}
• STACK_DESTROY: ${{ inputs.stack_destroy }}

As we need the public EC2 Instance IP for Ansible, the option to disable it makes no sense at all.

Should we delete the bucket if the stack-action is set to destroy?
Should we add another variable to handle deletion?

Something like this should do it, we could add this in the after Terraform scripts.

aws s3 rb s3://$TF_STATE_BUCKET --force

Need to provide cert and key to host ssl connections