Dev #1

Ishu27 · 2021-05-08T08:11:05Z

Addition of Event Templates

templates/workflow-template/workflow-template.yaml

aelbarkani · 2021-05-08T19:48:19Z

templates/events/event-bind.yaml

+apiVersion: argoproj.io/v1alpha1
+kind: WorkflowEventBinding
+metadata:
+        name: event-bind-workflow


too much indentation

Rectified Indentation

seems there is still too much indentation

This is the first commit (the outdated one ), I have made a different commit named "Minor Changes".

templates/secrets/secret-role-binding.yaml

aelbarkani · 2021-05-10T19:35:21Z

templates/secrets/secret-svc.yaml

+apiVersion: v1
+kind: ServiceAccount
+metadata:
+        name: github.com


too much indentation

aelbarkani · 2021-05-10T19:37:20Z

templates/workflow-template/workflow-template.yaml

+    container:
+      image: alpine/helm
+      command: [sh, -c]
+      args: ["apk add --update git; git clone {{ workflow.parameters.github }}; DIR_TEST=$(ls) && cd $DIR_TEST && ls && export HELM_EXPERIMENTAL_OCI=1 && echo {{ inputs.parameters.token }} | helm registry login --username AWS --password-stdin {{ workflow.parameters.registry }}; helm chart save . {{ workflow.parameters.imagename }} && helm chart push {{ workflow.parameters.imagename }}"]


apk add should not be used, nor any package manager, since the image should not be root so these command should not be able to work. so this step should be splitted into more steps, one with git image and another with helm.
And by the way, how did this work ? Did you use an SCC or something ?

Removed the apk and added an extra step for git . No, I did not use any SCC. This would just pull the repo package it and push it to the Registry. Also there needs to be a webhook created for the repository so that it could be the trigger point

aelbarkani · 2021-05-10T19:47:07Z

templates/workflow-template/workflow-template.yaml

+      value: <secret_key>
+    - name: access
+      value: <access_key>
+    - name: github


should be a generic git url, not only github

Removed it and passed it on k8s secret

aelbarkani · 2021-05-10T19:47:43Z

templates/workflow-template/workflow-template.yaml

+      value: <aws_region>
+    - name: registry
+      value: "<registry_name>"
+    - name: imagename


chart name instead of imagename

Changed to imagename

aelbarkani · 2021-05-10T19:49:06Z

templates/workflow-template/workflow-template.yaml

+
+  - name: gen-token-bash
+    script:
+      image: amazon/aws-cli


we should have an image with a tag (specific version), not latest

Made the image tag specific

aelbarkani · 2021-05-10T19:50:33Z

templates/workflow-template/workflow-template.yaml

+  templates:
+  - name: generate-token
+    steps:
+    - - name: generate


we need a more specific name for this step, "generate" is too generic

Changed generate to "create-token"

aelbarkani · 2021-05-10T19:51:54Z

templates/workflow-template/workflow-template.yaml

+      image: amazon/aws-cli
+      command: [bash]
+      source: |                                        
+        export AWS_ACCESS_KEY_ID={{ workflow.parameters.access }} && export AWS_SECRET_ACCESS_KEY={{ workflow.parameters.secret }} && export AWS_DEFAULT_REGION={{ workflow.parameters.region }} && /usr/local/bin/aws ecr-public get-login-password --region {{ workflow.parameters.region }}


there are several security issues here. secrets should not be exposed like this in CLI

Passed access-keys and all secrets into k8s secrets

aelbarkani · 2021-05-10T19:53:19Z

templates/workflow-template/workflow-template.yaml

+    container:
+      image: alpine/helm
+      command: [sh, -c]
+      args: ["apk add --update git; git clone {{ workflow.parameters.github }}; DIR_TEST=$(ls) && cd $DIR_TEST && ls && export HELM_EXPERIMENTAL_OCI=1 && echo {{ inputs.parameters.token }} | helm registry login --username AWS --password-stdin {{ workflow.parameters.registry }}; helm chart save . {{ workflow.parameters.imagename }} && helm chart push {{ workflow.parameters.imagename }}"]


DIR_TEST: is it really a directory for test ?

Removed DIR_TEST

aelbarkani · 2021-05-10T19:56:14Z

templates/workflow-template/workflow-template.yaml

+    container:
+      image: alpine/helm
+      command: [sh, -c]
+      args: ["apk add --update git; git clone {{ workflow.parameters.github }}; DIR_TEST=$(ls) && cd $DIR_TEST && ls && export HELM_EXPERIMENTAL_OCI=1 && echo {{ inputs.parameters.token }} | helm registry login --username AWS --password-stdin {{ workflow.parameters.registry }}; helm chart save . {{ workflow.parameters.imagename }} && helm chart push {{ workflow.parameters.imagename }}"]


how do you manage git clone git repos that are not public which needs an authentication ?

Have taken a user input if repo is private or public , if private then user will have to create a secret whose template also I have added.

aelbarkani · 2021-05-13T19:55:18Z

templates/workflow-template/workflow-template.yaml

+      value: private                              # Can be private or public accordingly, private creds to be stored in a secret .
+
+    - name: public-url
+      value: "https://github.com/Ishu27/test-argo-workflow.git"


these examples should be removed, we need something clean

Removed examples and changed variable to git-url

aelbarkani · 2021-05-13T19:55:26Z

templates/workflow-template/workflow-template.yaml

+      value: us-east-1
+
+    - name: registry
+      value: "public.ecr.aws/h8x3r6g1"


these examples should be removed, we need something clean

Removed example and added simple registry_name instead.

aelbarkani · 2021-05-13T19:55:31Z

templates/workflow-template/workflow-template.yaml

+    - name: registry
+      value: "public.ecr.aws/h8x3r6g1"
+    - name: chartname
+      value: "public.ecr.aws/h8x3r6g1/test-argo:v2"


this should be removed, we need something clean

Removed and renamed

aelbarkani · 2021-05-13T19:56:58Z

templates/workflow-template/workflow-template.yaml

+      - name: workdir
+        mountPath: /mnt/vol
+
+  - name: others


"others" is too generic. step names should be specific and self-explanatory about the function of that step, like you did for other steps.

Changed to private-registry

aelbarkani · 2021-05-13T19:57:58Z

templates/workflow-template/workflow-template.yaml

+metadata:
+  name: push-to-registry-template
+spec:
+  arguments:


is arguments field necessary or can it be removed ?

Parameters needs to be passed which are under arguments

aelbarkani · 2021-05-13T20:03:42Z

templates/workflow-template/workflow-template.yaml

+  templates:
+  - name: generate-token
+    steps:
+    - - name: pull-public-repo


this should be simplified. we should only have one "pull-repo" step, which should check in bash script whether git credentials have been provided. if yes, then they should be used. if not, then the script should assume that it's a public repo

Changed the code and used a single step pull-repo

aelbarkani · 2021-05-13T20:08:04Z

templates/workflow-template/workflow-template.yaml

+      image: alpine/helm:3.5.4
+      imagePullPolicy: IfNotPresent
+      command: [sh, -c]
+      args: ["export HELM_EXPERIMENTAL_OCI=1 && helm registry login {{ workflow.parameters.registry }} --username $registry_username --password $registry_password ; cd /mnt/vol/git-local-dir && helm chart save . {{ workflow.parameters.chartname }} && helm chart push {{ workflow.parameters.chartname }} "]


you can use "script" step to make the code more readable instead of using "&&" a lot. you can find an example here: https://argoproj.github.io/argo-workflows/examples/#scripts-results

Changed and used script

aelbarkani · 2021-05-13T20:08:16Z

templates/workflow-template/workflow-template.yaml

+      image: amazon/aws-cli:2.2.4
+      imagePullPolicy: IfNotPresent
+      command: [sh, -c]
+      args: ["export AWS_ACCESS_KEY_ID=$access_key && export AWS_SECRET_ACCESS_KEY=$secret_key && export AWS_DEFAULT_REGION={{ workflow.parameters.region }} && /usr/local/bin/aws ecr-public get-login-password --region {{ workflow.parameters.region }}"]


you can use "script" step to make the code more readable instead of using "&&" a lot. you can find an example here: https://argoproj.github.io/argo-workflows/examples/#scripts-results

changed and used script

aelbarkani · 2021-05-13T20:08:23Z

templates/workflow-template/workflow-template.yaml

+      image: alpine/helm:3.5.4
+      imagePullPolicy: IfNotPresent
+      command: [sh, -c]
+      args: ["export HELM_EXPERIMENTAL_OCI=1 && echo {{ inputs.parameters.token }} | helm registry login --username AWS --password-stdin {{ workflow.parameters.registry }}; cd /mnt/vol/git-local-dir && helm chart save . {{ workflow.parameters.chartname }} && helm chart push {{ workflow.parameters.chartname }}"]


you can use "script" step to make the code more readable instead of using "&&" a lot. you can find an example here: https://argoproj.github.io/argo-workflows/examples/#scripts-results

root added 2 commits May 8, 2021 07:54

Dev Branch

e4980bf

with README

84d8d9b

Ishu27 requested a review from aelbarkani May 8, 2021 08:31

aelbarkani requested changes May 8, 2021

View reviewed changes

Multi-Step-Workflow

cc23e48

Ishu27 requested a review from aelbarkani May 10, 2021 09:34

Minor Changes

bb64471

aelbarkani requested changes May 10, 2021

View reviewed changes

root added 2 commits May 13, 2021 05:24

Changes in Workflow-Template

db2b364

Removed Namespace

c750e3f

Ishu27 requested a review from aelbarkani May 13, 2021 05:37

aelbarkani requested changes May 13, 2021

View reviewed changes

root added 2 commits May 14, 2021 10:31

Addition of Script

7e543d3

Removed _workflow-template

13df602

Ishu27 requested a review from aelbarkani May 14, 2021 10:35

Dev #1

Are you sure you want to change the base?

Dev #1

Conversation

Ishu27 commented May 8, 2021

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

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

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

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