WIP - Merge Une-T INS improvements #5

franck-boullier · 2020-01-30T10:39:39Z

This is needed so that the codebase for Unee-T is easier to deploy on different installation.

We are importing some of the improvements made in the Unee-T INS fork.
We are also making sure that we are using variables instead of hardcoded values as much as possible

* Add some missing variables * build on remove_hard_coded_variables branch * add excute permission for deploy.sh * Install AWS CLI * Fix AWS credentials * Change build on master branch

Import changes and improvements from Unee-T INS

…sing names for key variables

franck-boullier · 2020-01-30T10:46:24Z

Pre-requisite:

Dependencies:

We MUST have reviewed, merged and tag released version 0.2.2 of the https://github.com/unee-t/env codebase.
See PR unee-t/env#2
DONE

AWS variables in the parameter store:

We MUST have the correct values for the following variables in the AWS parameter store for each environment: DEV (DONE), PROD (DONE), DEMO (DONE)

DOMAIN
INSTALLATION_ID
STAGE
EMAIL_FOR_NOTIFICATION_APIENROLL
PRIVATE_SUBNET_1
PRIVATE_SUBNET_2
PRIVATE_SUBNET_3
LAMBDA_TO_RDS_SECURITY_GROUP
API_ACCESS_TOKEN
DONE

Travis CI Setting:

We MUST have the correct values for the following variables in the AWS parameter store for each environment: DEV (DONE), PROD (DONE), DEMO (DONE)

DOCKER_USERNAME
DOCKER_PASSWORD
AWS_DEFAULT_REGION
AWS_ACCOUNT_USER_ID_DEV
AWS_ACCOUNT_SECRET_DEV
AWS_PROFILE_DEV
AWS_ACCOUNT_USER_ID_DEMO
AWS_ACCOUNT_SECRET_DEMO
AWS_PROFILE_DEMO
AWS_ACCOUNT_USER_ID_PROD
AWS_ACCOUNT_SECRET_PROD
AWS_PROFILE_PROD
GITHUB_TOKEN

DONE

Issues that are NOT fixed yet:

See #3 and #4

We need to have a solution for these before we merge.

kaihendry · 2020-01-30T12:25:11Z

test.sh

 curl -i -X POST \
  http://localhost:3000 \
-  -H "Authorization: Bearer $(aws --profile uneet-dev ssm get-parameters --names API_ACCESS_TOKEN --with-decryption --query Parameters[0].Value --output text)" \
+  -H "Authorization: Bearer $(aws --profile ins-dev ssm get-parameters --names API_ACCESS_TOKEN --with-decryption --query Parameters[0].Value --output text)" \


Why ins here?

@kaihendry ins is there because we shoulde replace this with a variable but I don't know how we could to that,
See #3 for more details

kaihendry · 2020-01-30T12:26:39Z

main.go

+//	- Stage (AWS parameter `STAGE`) 
+// The value for "ins-dev" is also declared in Travis Setting
+// it is then passed on as the variable `TRAVIS_PROFILE`
+	cfg, err := external.LoadDefaultAWSConfig(external.WithSharedConfigProfile("ins-dev"))


The way this line works, is that it uses "ins-dev" locally, else it uses the current execution environment.

So imo it should be set to the environment you develop in. E.g. uneet-dev in this case

So imo it should be set to the environment you develop in. E.g. uneet-dev in this case

The problem with this approach is that this means the code will not be usable "as is" in the Unee-T INS fork <-- this will make things more obscure as the people in charge on the Unee-T INS code will have to know that this repo has to be forked and modified in order for their installation to work as intended.

Much better to use a variable that is already declared in the AWS parameter store and in the Travis settings instead.

See #4 for more details

go.mod

kaihendry · 2020-01-30T12:29:04Z

deploy.sh

@@ -0,0 +1,47 @@
+#!/bin/bash


Why not configure the AWS environment in travis.yml?

Why not configure the AWS environment in travis.yml?

Because I'm not sure how this can be done.
Feel free to suggest an alternative to how things are done today

It was shown how to do it here: https://github.com/Unee-T-INS/lambda2sqs/blob/master/.travis.yml

kaihendry · 2020-01-30T12:31:17Z

Makefile

+# We create a function to simplify getting variables for aws parameter store.
+
+define ssm
+$(shell aws --profile $(TRAVIS_PROFILE) ssm get-parameters --names $1 --with-decryption --query Parameters[0].Value --output text)


What's the default TRAVIS_PROFILE?

Why is it called TRAVIS_PROFILE and not.... AWS_PROFILE?

What's the default TRAVIS_PROFILE?

See the code in the travis.yml file on line 46, 53 and 61

Why is it called TRAVIS_PROFILE and not.... AWS_PROFILE?

Because that variable is NOT coming from AWS, that variable is defined when travis.yml runs and because its value will be based on the Travis CI settings for this environment.

The idea with a Makefile and most other robust software is that you define a default that can be overridden.

These are defaults: https://github.com/Unee-T-INS/lambda2sqs/blob/master/Makefile#L1-L5 and the style you should emulate here, else when you run make it won't work.

The idea with a Makefile and most other robust software is that you define a default that can be overridden.

These are defaults: https://github.com/Unee-T-INS/lambda2sqs/blob/master/Makefile#L1-L5 and the style you should emulate here, else when you run make it won't work.

I understand the idea but we've not been able to implement that using variables instaed of hardcoded values.
In your example: https://github.com/Unee-T-INS/lambda2sqs/blob/master/Makefile#L1-L5
We need to replace the hardcoded values

PROJECT = lambda2sqs <--- this is OK since this is the hardcoded name of the project STACK_NAME ?= $(PROJECT) <--- this is OK (it's a variable) AWS_REGION = ap-southeast-1 <--- this does NOT seem to be OK: it should be a variable DEPLOY_S3_PREFIX = lambda2sqs <--- this is OK since this is a hardcoded prefix related to the name of the project (question: why not use the existing PROJECT variable since the value is the same?) AWS_PROFILE = ins-dev <--- this does NOT seem to be OK: it should be a variable

These are variables that can be overridden.

DEPLOY_S3_PREFIX can be the PROJECT variable.

These are variables that can be overridden.

I'm sure this is true but you've never explained HOW they can ACTUALLY be overridden...
If you can do that (show HOW they can be overridden) then I think we'll have made huge progress.

Sorry I thought I showed in the call / screen share.

[hendry@t480s m]$ make foobar [hendry@t480s m]$ make A_VARIABLE=somethingelse somethingelse [hendry@t480s m]$ cat Makefile A_VARIABLE := foobar all: @echo $(A_VARIABLE)

…usion

franck-boullier · 2020-01-31T03:44:20Z

Build is failing with this error:

@kaihendry mentioned in some of our conversation that this might be solved by setting a parameter GITHUB_TOKEN in the Travis CI setting for this repo

The problem at this stage are:

there IS a parameter GITHUB_TOKEN already setup in the Travis Settings for this Repo.
We have no information about where/how we can find the value for this variable GITHUB_TOKEN <-- we are unable to update it.

@kaihendry more information about how/where we can find the value for the GITHUB_TOKEN variable will be helpful.

kaihendry · 2020-01-31T04:30:46Z

https://help.github.com/en/github/authenticating-to-github/creating-a-personal-access-token-for-the-command-line I believe shows how to create a GITHUB_TOKEN

franck-boullier · 2020-01-31T04:43:04Z

https://help.github.com/en/github/authenticating-to-github/creating-a-personal-access-token-for-the-command-line I believe shows how to create a GITHUB_TOKEN

Which one do I need to choose?

kaihendry · 2020-01-31T04:47:53Z

You don't need to choose any, the idea with GITHUB_TOKEN is that just identifies you to facilitate the download. It doesn't need capabilities to access repos IIRC.

You could just copy it from the unee-t travis configuration too btw.

…ys be specific and descriptive

… specific and descriptive

… be specific and descriptive

… and descriptive

… be explicit

…ployed

…in.go file

kaihendry and others added 13 commits January 11, 2020 16:50

Convert to support ins-dev environment

7ef8483

Remove hard coded variables (#2)

54717fd

* Add some missing variables * build on remove_hard_coded_variables branch * add excute permission for deploy.sh * Install AWS CLI * Fix AWS credentials * Change build on master branch

Merge pull request #2 from Unee-T-INS/master

1aa7fad

Import changes and improvements from Unee-T INS

add comments, remove unnecessary step, make sure PATH is correct

54be7d1

use the update variables names from Travis CI

89a4101

make sure we use the main env codebase as a dependency

6447e93

make sure we use the main env codebase as a dependency

3f8cfbb

make sure we use the main env codebase as a dependency

661b366

add comments - make sure we can deploy DEMO and PROD - use less confu…

4713000

…sing names for key variables

improve documentation

23c53b7

Lint - improve readability

074234b

add more info to the documentation

a185f91

highlight the code that we need to correct before merge

2616b17

franck-boullier requested a review from kaihendry January 30, 2020 10:39

kaihendry reviewed Jan 30, 2020

View reviewed changes

franck added 3 commits January 31, 2020 10:48

linting

772294f

rename variable TRAVIS_PROFILE to TRAVIS_AWS_PROFILE to minimize conf…

af1f8e0

…usion

clarify how deployments are done

d7fc311

franck added 7 commits January 31, 2020 16:29

update go modules

6a0d75e

add more details to the documentation

1e5235d

use environment variables to define the AWS user profile to use for test

f99f9a5

use environment variable to get the valued of the region

f18cf8e

do not use deploy.sh script

8a03b5d

WIP - use hardcoded value for the AWS profile

80b9759

test Travis deployment

646b491

franck added 17 commits February 5, 2020 19:42

rename type from handler to handlerBzDbConnexion name should alwa…

0338216

…ys be specific and descriptive

rename type from EnvCode to environmentCode name should always be…

e0a75b6

… specific and descriptive

rename Named return from e to thisEnvironment names should always…

9eb3d0f

… be specific and descriptive

some Named return changes were omitted - do these

c02119b

rename from Code to environmentId names should always be specific…

4ef76d3

… and descriptive

remove unnecessary type environmentCode

d9e7136

use a more appropriate type name handlerSqlConnexion

5b9fd89

use a more appropriate type name handlerSqlConnexion

bd73245

replace confusing name h with currentBzConnexion and bzDbConnexion

efe7b1c

replace confusing name e with thisEnvironment names should always…

4d8e9d5

… be explicit

replace confusing name e with thisEnvironment names should always…

591a9e5

… be explicit

make sure we get variables from the parameter store when Lambda is de…

8f6afd4

…ployed

fix typo so we can get the STAGE for this installation

9f15390

improve log for easier debugging

5613835

more log message to facilitate debugging

c07c682

more info for logs

522af86

add comment to try to understand how this works...

b8192a1

franck-boullier mentioned this pull request Feb 5, 2020

Error with Protect function #7

Open

franck added 12 commits February 6, 2020 00:08

do not inadvertently display sensitive data in the log

d9661dd

add more comment in the code to facilitate debugging in the future

be38869

add more info to the documentation

cb3f29a

more information on the doc on logs and debugging

a7b3f8e

no need for GoReleaser because local deployment are irrelevant

8db48ea

upgrade modules to use unee-t/env v1.0.0

4f80417

upgrade to Unee-t/env v1.0.1

87815ff

cleanup go.sum

1202d63

upgrade to Unee-t/env v1.0.2

888adb2

upgrade to Unee-t/env v1.0.3

c70359e

import code from unee-t/env to make sure we have no error in the ma…

8276a57

…in.go file

add more details in the error messages for easier debugging

2879bd4

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

WIP - Merge Une-T INS improvements #5

WIP - Merge Une-T INS improvements #5

franck-boullier commented Jan 30, 2020

franck-boullier commented Jan 30, 2020 •

edited

Loading

kaihendry Jan 30, 2020

franck-boullier Jan 31, 2020

kaihendry Jan 30, 2020

franck-boullier Jan 31, 2020

kaihendry Jan 30, 2020

franck-boullier Jan 31, 2020

kaihendry Jan 31, 2020

kaihendry Jan 30, 2020

franck-boullier Jan 31, 2020

kaihendry Jan 31, 2020

franck-boullier Jan 31, 2020 •

edited

Loading

kaihendry Jan 31, 2020 •

edited

Loading

franck-boullier Jan 31, 2020

kaihendry Jan 31, 2020

franck-boullier commented Jan 31, 2020 •

edited

Loading

kaihendry commented Jan 31, 2020

franck-boullier commented Jan 31, 2020

kaihendry commented Jan 31, 2020

WIP - Merge Une-T INS improvements #5

Are you sure you want to change the base?

WIP - Merge Une-T INS improvements #5

Conversation

franck-boullier commented Jan 30, 2020

franck-boullier commented Jan 30, 2020 • edited Loading

Pre-requisite:

Dependencies:

AWS variables in the parameter store:

Travis CI Setting:

Issues that are NOT fixed yet:

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

franck-boullier Jan 31, 2020 • edited Loading

Choose a reason for hiding this comment

kaihendry Jan 31, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

franck-boullier commented Jan 31, 2020 • edited Loading

kaihendry commented Jan 31, 2020

franck-boullier commented Jan 31, 2020

kaihendry commented Jan 31, 2020

franck-boullier commented Jan 30, 2020 •

edited

Loading

franck-boullier Jan 31, 2020 •

edited

Loading

kaihendry Jan 31, 2020 •

edited

Loading

franck-boullier commented Jan 31, 2020 •

edited

Loading