Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cloud9 cfn template #198

Closed
wants to merge 8 commits into from
Closed

Cloud9 cfn template #198

wants to merge 8 commits into from

Conversation

snjkumar23
Copy link
Contributor

What does this PR do and why?

Cloud9 CF template Integration

Issue #, if available

PR acceptance checklist

This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.

  • [ Y] Added/Updated documentation if applicable.
  • [ Y] Pre-commit checks passed.
  • [ Y] Lint and Nag checks passed.
  • If releasing a new version, have you bumped the version make version part=<major|minor|patch>?

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@snjkumar23 snjkumar23 requested a review from a team as a code owner April 13, 2023 17:28
Copy link
Contributor

@rezabekf rezabekf left a comment

Choose a reason for hiding this comment

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

I would recommend to remove cfn Cloud9 support as the template needs quite a bit of work. I would suggest to move the part where we create C9 via CFN to own PR so we can go and publish these pre-req.

This will allow us to move forward with individual labs and test them in C9 created in console.
The automation part, even thou is nice to have and will definitely help out, we need to make sure it is not blocking us moving forward

Comment on lines 3 to 5
Description:
AWS CloudFormation template for dynamic Cloud 9 setups. Creates a Cloud9
bootstraps the instance.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Description:
AWS CloudFormation template for dynamic Cloud 9 setups. Creates a Cloud9
bootstraps the instance.
Description:
AWS CloudFormation workshop - Cloud9 IDE (uksb-1q9p31idr).

Comment on lines 7 to 9
BucketName:
Type: String
Default: "cfn-workshop-01-123456789 "
Copy link
Contributor

Choose a reason for hiding this comment

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

We should not explicitly name buckets. Let cfn to generate name and output the value in Outputs.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We should not explicitly name buckets. Let cfn to generate name and output the value in Outputs.

Will handle it in new pr

Comment on lines 14 to 19
AllowedValues:
- t2.micro
- t3.micro
- t3.small
- t3.medium
ConstraintDescription: Must be a valid Cloud9 instance type
Copy link
Contributor

Choose a reason for hiding this comment

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

we should not restrict to only t instances, either support all here or I would remove and let user to decide what they want. I am suggesting t3.micro as default as it is free tier.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

we should not restrict to only t instances, either support all here or I would remove and let user to decide what they want. I am suggesting t3.micro as default as it is free tier.

will remove list and default to t3.micro in new pr

C9InstanceType:
Description: Cloud9 instance type
Type: String
Default: t3.small
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Default: t3.small
Default: t3.micro

Copy link
Contributor

Choose a reason for hiding this comment

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

small is not a free tier

Comment on lines 20 to 27
C9EnvType:
Description: Environment type.
Default: self
Type: String
AllowedValues:
- self
- 3rdParty
ConstraintDescription: must specify self or 3rdParty.
Copy link
Contributor

Choose a reason for hiding this comment

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

What does this does? I didn't see anything in deployment instructions what self and 3rd party means...do we need user to make choice if we are saying leave all properties default?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What does this does? I didn't see anything in deployment instructions what self and 3rd party means...do we need user to make choice if we are saying leave all properties default?

we do not need any of these

Copy link
Contributor

Choose a reason for hiding this comment

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

this doesnt seem to be used anywhere?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

this doesnt seem to be used anywhere?

yes, earlier this page had images, not needed anymore, removed

Copy link
Contributor

Choose a reason for hiding this comment

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

this doesn't seem to be used anywhere?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

this doesn't seem to be used anywhere?

yes, earlier this page had images, not needed anymore, removed

Copy link
Contributor

Choose a reason for hiding this comment

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

this doesn't seem to be used anywhere?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

this doesn't seem to be used anywhere?

yes, earlier this page had images, not needed anymore, removed

Copy link
Contributor

Choose a reason for hiding this comment

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

this doesn't seem to be used anywhere?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

this doesn't seem to be used anywhere?

yes, earlier this page had images, not needed anymore, removed

Copy link
Contributor

Choose a reason for hiding this comment

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

this doesn't seem to be used anywhere?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

this doesn't seem to be used anywhere?

yes, earlier this page had images, not needed anymore, removed

@rezabekf
Copy link
Contributor

I have moved cloud9 folder to solutions to enable running automated checks. I have fixed some of the issues to be able to commit. It doesn't look all that bad, once we fix cfn-nag issues, but I still think we should make the automation part separate PR to unblock us....

Copy link
Contributor

@rezabekf rezabekf left a comment

Choose a reason for hiding this comment

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

Oveall, this is very good. Few nit picks and little reorganization to improve Pre-req section.


[AWS Cloud9](https://aws.amazon.com/cloud9/) is a cloud-based IDE that lets you write, run, and debug your code with just a browser. It includes a code editor, debugger, and terminal. Cloud9 comes prepackaged with essential tools for running this workshop. Since your Cloud9 IDE is cloud-based, you can do labs from your office, home, or anywhere using an internet-connected machine.

We recommend using it to run this workshop because it comes with the necessary set of tools pre-installed. If you prefer to work locally, the next section includes details on how to setup your local development environment.
Copy link
Contributor

Choose a reason for hiding this comment

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

We need to be more specific here. Out of the box the C9 comes with following tools relevant to the workshop:

  • Pyhton
  • pip
  • awscli


[AWS Cloud9](https://aws.amazon.com/cloud9/) is a cloud-based IDE that lets you write, run, and debug your code with just a browser. It includes a code editor, debugger, and terminal. Cloud9 comes prepackaged with essential tools for running this workshop. Since your Cloud9 IDE is cloud-based, you can do labs from your office, home, or anywhere using an internet-connected machine.

We recommend using it to run this workshop because it comes with the necessary set of tools pre-installed. If you prefer to work locally, the next section includes details on how to setup your local development environment.
Copy link
Contributor

Choose a reason for hiding this comment

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

I think the should reword little bit. Something like, "For the best experience with the workshop and minimal setup effort, we recommend..."

We recommend using **us-east-1 (N. Virginia)** as the _AWS Region_ for the workshop.
:::
1. Once created, your instance should be listed on the AWS Cloud9 [Environments](https://console.aws.amazon.com/cloud9/home) page. Click **Open** if not already.
1. You will see a terminal area at the bottom where you will run the commands as you progress through the workshop. In the main work area you will open and edit code and template files. AWS Cloud9 comes with the required command line tools (aws cli, python) pre-installed.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
1. You will see a terminal area at the bottom where you will run the commands as you progress through the workshop. In the main work area you will open and edit code and template files. AWS Cloud9 comes with the required command line tools (aws cli, python) pre-installed.
1. You will see a terminal area at the bottom where you will run the commands as you progress through the workshop. In the main work area you will open and edit code and template files.

Comment on lines 20 to 23
1. There is one tool to update, run below command on terminal to update the **pip** version:
:::code{language=shell showLineNumbers=false showCopyAction=true}
sudo pip install --upgrade pip
:::
Copy link
Contributor

Choose a reason for hiding this comment

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

why we upgrading pip? also running sudo is not a good practices with a pip...it should be run as a user rather root...

Copy link
Contributor

Choose a reason for hiding this comment

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

maybe as a final step customers can test that aws cli is working by running aws s3 ls...i think that's all needed at this stage?

Copy link
Contributor

Choose a reason for hiding this comment

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

actually we should have section for cloning the repo here as it is different to local...and we can move git instructions to the local development

Copy link
Contributor

Choose a reason for hiding this comment

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

I have notice that you have updated Get lab resources section...lets move it here and we can move rest of that file to local development

Copy link
Contributor Author

Choose a reason for hiding this comment

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

why we upgrading pip? also running sudo is not a good practices with a pip...it should be run as a user rather root...

without pip upgrade pip install cfn-lint do not work as default pip is older version and fails while installing cfn-lint

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have notice that you have updated Get lab resources section...lets move it here and we can move rest of that file to local development

done, please have a look and share your suggestions

Copy link
Contributor

Choose a reason for hiding this comment

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

lets move this to c9 content

Copy link
Contributor

Choose a reason for hiding this comment

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

lest get rid of this file and move relevant content to C9 and LocalDev as appropriate...that will simplify pre-req section a lot. We will have folowing structure:

  • Create an AWS Account
  • Use Cloud9 IDE
  • Use Local Development
  • Default VPC

@@ -0,0 +1,96 @@
---
title: "Use local development"
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
title: "Use local development"
title: "Use Local Development"

@rezabekf
Copy link
Contributor

Closing this one in favor of #215

@rezabekf rezabekf closed this Apr 25, 2023
@rezabekf rezabekf deleted the cloud9-cfn-template branch May 22, 2023 11:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants