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

New Task: Argo CD #49

Merged
merged 4 commits into from
Jun 26, 2019
Merged

New Task: Argo CD #49

merged 4 commits into from
Jun 26, 2019

Conversation

AdityaGupta1
Copy link
Contributor

@AdityaGupta1 AdityaGupta1 commented Jun 19, 2019

Changes

Created a Task that syncs (deploys) an Argo CD application.

Submitter Checklist

These are the criteria that every PR should meet, please check them off as you
review them:

See the contribution guide
for more details.

This task is used to sync an Argo CD application with its underlying Git repository and to wait for it to become healthy. It takes in no resources, only parameters, and passes them to the argocd command. The application's name and the Argo CD server address are required. Some form of login is also required, either through a username/password combo or an authentication token. The specific revision to sync to and the flags to append to commands can also be passed as parameters.

Currently there is no way to pass plain strings or files between tasks, or it could have been better to split the login and sync tasks. This way, there could have been two login tasks - one for username/password and one for authentication token. With the current approach, the bash command runs an if statement to see if the token's default value was overriden, and if so, it uses that token. Otherwise, it uses the username/password passed in. This issue (typing of PipelineResources) is being discussed in tektoncd/pipeline#238.
@googlebot
Copy link

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here (e.g. I signed it!) and we'll verify it.


What to do if you already signed the CLA

Individual signers
Corporate signers

ℹ️ Googlers: Go here for more info.

@tekton-robot tekton-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Jun 19, 2019
@AdityaGupta1
Copy link
Contributor Author

I signed the CLA

@googlebot
Copy link

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

Copy link
Member

@vdemeester vdemeester left a comment

Choose a reason for hiding this comment

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

@AdityaGupta1 Does argocd supports reading authentication from a file. I really think the auth should be passed as secrets (either using env and envFrom with secrets or using mounted secret in the Task steps)

argocd/argocd.yaml Outdated Show resolved Hide resolved
argocd/argocd.yaml Outdated Show resolved Hide resolved
argocd/argocd.yaml Outdated Show resolved Hide resolved
@AdityaGupta1
Copy link
Contributor Author

@vdemeester would something like the following work?

apiVersion: v1
kind: ConfigMap
metadata:
  name: env-configmap
data:
  ARGOCD_SERVER: <server address>
---
apiVersion: v1
kind: Secret
metadata:
  name: env-secret
data:
  ARGOCD_USERNAME: <base64 encoded username>
  ARGOCD_PASSWORD: <base64 encoded password>
  ARGOCD_AUTH_TOKEN: <base64 encoded token or empty>
---
apiVersion: tekton.dev/v1alpha1
kind: Task
metadata:
  name: argocd-task-sync-and-wait
spec:
  inputs:
    params:
      - name: application-name
        description: name of the application to sync
      - name: revision
        description: the revision to sync to
        default: HEAD
      - name: flags
        default: --
  containerTemplate:
    envFrom:
      - configMapRef:
          name: env-configmap
      - secretRef:
          name: env-secret
  steps:
    - name: login
      image: argoproj/argocd
      command: ["/bin/bash", "-c"]
      args:
        - if [ -z $ARGOCD_AUTH_TOKEN ]; then
            yes | argocd login $ARGOCD_SERVER --username=ARGOCD_USERNAME --password=ARGOCD_PASSWORD;
          fi
    - name: sync
      image: argoproj/argocd
      command: ["/bin/bash", "-c"]
      args:
        - argocd app sync ${inputs.params.application-name} --revision ${inputs.params.revision} ${inputs.params.flags}
    - name: wait
      image: argoproj/argocd
      command: ["/bin/bash", "-c"]
      args:
        - argocd app wait ${inputs.params.application-name} --health ${inputs.params.flags}

If this is what works, I'll commit this along with an updated README with instructions on how to use the ConfigMap and Secret for authentication.

@dlorenc
Copy link
Contributor

dlorenc commented Jun 26, 2019

/ok-to-test

@tekton-robot tekton-robot removed the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Jun 26, 2019
@dlorenc
Copy link
Contributor

dlorenc commented Jun 26, 2019

If this is what works, I'll commit this along with an updated README with instructions on how to use the ConfigMap and Secret for authentication.

Yeah this makes sense!

@vdemeester
Copy link
Member

    envFrom:
      - configMapRef:
          name: env-configmap
      - secretRef:
          name: env-secret

Those could even be parametrize, aka having a parameter for the task and a default value, letting users use the secret name the want. But it will only work on 0.5.0 (as containerTemplate doesn't support template before tektoncd/pipeline#1006 is merged).

@AdityaGupta1
Copy link
Contributor Author

@dlorenc I've updated it to use ConfigMap and Secret, and the instructions have been updated to match

@tekton-robot tekton-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jun 26, 2019
@dlorenc
Copy link
Contributor

dlorenc commented Jun 26, 2019

/lgtm

@tekton-robot tekton-robot added the lgtm Indicates that a PR is ready to be merged. label Jun 26, 2019
@dlorenc
Copy link
Contributor

dlorenc commented Jun 26, 2019

/approve

@tekton-robot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: AdityaGupta1, dlorenc

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@tekton-robot tekton-robot merged commit 667ccf0 into tektoncd:master Jun 26, 2019
@AdityaGupta1
Copy link
Contributor Author

AdityaGupta1 commented Jun 28, 2019

It seems like there are some bugs with the latest version of Argo CD that prevent it from working, but my internship at Intuit ends today and I doubt I'll have time to work on it afterwards. Reverting to container version 1.0.2 solves the problem for username/password login, but there is still a bug with token login that is being looked into.

@AdityaGupta1
Copy link
Contributor Author

@vdemeester @dlorenc I just noticed there is a typo in the login command, in that it is missing a $ before ARGOCD_USERNAME and ARGOCD_PASSWORD. Should I submit another pull request to fix that?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants