Skip to content

Weekly environment - Vanilla POC#746

Merged
ofiriro3 merged 110 commits into
elastic:mainfrom
ofiriro3:vanilla_poc
Feb 23, 2023
Merged

Weekly environment - Vanilla POC#746
ofiriro3 merged 110 commits into
elastic:mainfrom
ofiriro3:vanilla_poc

Conversation

@ofiriro3

@ofiriro3 ofiriro3 commented Feb 14, 2023

Copy link
Copy Markdown
Contributor

Summary of your changes

This is the first item in the development of the Daily-environment epic.

In this task, we created a job that deploys a vanilla KSPM integration on an Elastic cloud environment.
The job currently uses a pre-exited EC2 machine and it deploys an agent there (no new EC2 machines will be created).

The expected behavior is:

  1. A new ELK environment is created with each execution.
  2. A new Vanilla integration is installed on this environment.
  3. The same EC2 machine whose credentials exist in the GitHub secret will deploy the agent.
  4. A slack message will be sent to ofir-personal-channel with the relevant credentials for this environment.

How to check the daily job
Currently, the flow is deployed using a dispatch workflow, meaning - you will need to apply it using GitHub actions.
You can apply the flow from my fork, using actions.
Please set the branch to vanilla_poc before running the dispatch workflow.
It should look like this:
image

The flow will deploy the integration to https://cloud.elastic.co/home, in the next PRs we will change it to a staging environment and replace the latest image with the SNAPSHOT version.

Related Issues

Related https://github.com/elastic/security-team/issues/5903

Checklist

  • I have added tests that prove my fix is effective or that my feature works
  • I have added the necessary README/documentation (if appropriate)

@ofiriro3
ofiriro3 marked this pull request as ready for review February 15, 2023 09:31

@oren-zohar oren-zohar left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only went over the workflow

Comment thread .github/workflows/weekly-enviroment.yml Outdated
TF_VAR_ec_api_key: ${{ secrets.WEEKLY_ENVIRONMENT_EC_API_KEY }}
TF_VAR_environment: ${{ github.event.inputs.logLevel }}
TF_LOG: ${{ github.event.inputs.logLevel }}
TF_VAR: 8.6.1

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is it hard coded by design? can we use the branch version instead?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, we would like the weekly environment to load a specific version every time it runs.

We would like it to run the latest SNAPSHOT version available. But we would like to change it manually every time since we would like to update the integration and the relevant rules.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure I understand the logic behind it. Why not use the branch version as a default value?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So just to verify that I understood you, you meant for example if the Cloudbeat version is 8.8.0, so deploy 8.8.0-SNAPSHOT?

So that is a great idea, but for the matter of this PR, we wanted to start with a POC that will use a hard-coded version.
In the future we would like even to support multiple versions:

But for this POC, we assumed that it uses a hard-coded version 8.6.1 as the current Terraform default value suggests.

Comment thread .github/workflows/weekly-enviroment.yml Outdated
Comment thread .github/workflows/weekly-enviroment.yml Outdated
- name: Setup Terraform
uses: hashicorp/setup-terraform@v2
with:
terraform_version: 1.3.5

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

don't we have the terraform version as part of hermit? I think we need to work with it in the CI, as it will make sure it will work properly locally for everyone in the future. What do you think?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

- name: Init Hermit

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested it, good advice! I removed the Terraform setup step and added a Init Hermit step

Comment thread .github/workflows/weekly-enviroment.yml Outdated
Comment on lines +48 to +59
- name: Terraform fmt
id: fmt
run: terraform fmt -check
continue-on-error: true

- name: Terraform Init
id: init
run: terraform init -no-color

- name: Terraform Validate
id: validate
run: terraform validate -no-color

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! but I think the linting should be in a separate step, as it is not part of the deployment logic, can run in parallel, and on failure, easier to understand what part failed and when.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I removed it 👍

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I still think it's valuable to have it - just in a separate step, maybe in one of the other workflows where we run linters?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done 👍

Comment thread .github/workflows/weekly-enviroment.yml Outdated
Comment thread .github/workflows/weekly-enviroment.yml Outdated
@olegsu

olegsu commented Feb 16, 2023

Copy link
Copy Markdown
Collaborator

Do we plan to consolidate it to the other terraform module at some point? what are the requirementes?
If not, why?

@ofiriro3

ofiriro3 commented Feb 20, 2023

Copy link
Copy Markdown
Contributor Author

Do we plan to consolidate it to the other terraform module at some point? what are the requirements?

Currently, the weekly environment job uses terraform to create a new Elastic cloud environment and does not require any other terraform modules from our solutions. Other actions are done by ssh to the relevant EC2 machine and applying the specific operations there.

For the first deliverable solution, we decided that the job won't create a new EC2 machine and its infrastructure, and instead will use an existing machine. This assumption was made to enable us to move fast with a reliable solution and not be dependent on the rest of the terraform modules. So in this case, I thought that doing the ssh will be done much easier in a bash script.

As I see it, in the future we will might decide to consolidate it with other terraform modules.
For example, we will might decide to create an EC2 machine if such does not exist as this ticket suggests - https://github.com/elastic/security-team/issues/5788, but before doing so, we will have to reconsider some of the dependencies we currently have:

  1. EC2 creation + agent deployment - we would like to deploy an agent on every job execution, but not to deploy a new EC2 every time.
  2. EKS creation + agent deployment - same as 1.
  3. API module - I guess that this entire module can be refactored.

@olegsu

olegsu commented Feb 21, 2023

Copy link
Copy Markdown
Collaborator

Thank you for a detailed explanation, @ofiriro3.

Currently, the weekly environment job uses terraform to create a new Elastic cloud environment and does not require any other terraform modules from our solutions. Other actions are done by ssh to the relevant EC2 machine and applying the specific operations there.

Imo, this is not ideal to have multiple scripts doing practically the same thing. I think a consolidation should be added to #622, not to DOD but to something that we want to track and achieve when possible. Otherwise, it would be hard to maintain two scripts.

@ofiriro3

Copy link
Copy Markdown
Contributor Author

#622, not to DOD but to something that we want to track and achieve when possible. Otherwise, it would be hard to maintain two scripts.

Done. I commented on the ticket

@olegsu olegsu left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good!
What happens if the machine does not exist?
Does the slack message still goes to ofir-personal-channel ?

- name: Init Hermit
run: ./bin/hermit env -r >> $GITHUB_ENV

- name: Terraform fmt

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lets have it atm only on the weekly env dir

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't mind changing it, but why we wouldn't want it to run on the entire solution?

The CI passes, and it will keep all the terraform in format.

env:
WORKING_DIR: deploy/weekly-environment
SCRIPTS_DIR: deploy/weekly-environment/scripts/benchmarks/kspm_vanilla
TF_VAR_ec_api_key: ${{ secrets.WEEKLY_ENVIRONMENT_EC_API_KEY }}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Who is the user behind this key?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Currently its my user, we have two tickets related:

  1. Short term solution - https://github.com/elastic/security-team/issues/6017, add the entire team to have permission on my account.

  2. Long term solution - Create a team account - https://github.com/elastic/security-team/issues/5921

@ofiriro3

Copy link
Copy Markdown
Contributor Author

Looks good!
What happens if the machine does not exist?
Does the slack message still goes to ofir-personal-channel ?

If the machine does not exist, no message is sent to the slack channel, but the job fails.

@ofiriro3
ofiriro3 requested a review from olegsu February 22, 2023 12:45
@ofiriro3
ofiriro3 enabled auto-merge (squash) February 23, 2023 09:24
@ofiriro3
ofiriro3 removed the request for review from kfirpeled February 23, 2023 09:24
@ofiriro3
ofiriro3 disabled auto-merge February 23, 2023 09:24
@ofiriro3
ofiriro3 enabled auto-merge (squash) February 23, 2023 09:25
@ofiriro3
ofiriro3 disabled auto-merge February 23, 2023 10:25
@ofiriro3
ofiriro3 merged commit f217bc8 into elastic:main Feb 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants