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

Support CRON trigger type for pipeline #39

Closed
dustinvanbuskirk opened this issue Mar 18, 2021 · 13 comments · Fixed by #90
Closed

Support CRON trigger type for pipeline #39

dustinvanbuskirk opened this issue Mar 18, 2021 · 13 comments · Fixed by #90
Labels
enhancement New feature or request

Comments

@dustinvanbuskirk
Copy link

Pipeline does not support CRON triggers.

We need to add this additional trigger type to support pipelines using CRON triggers.

@templarfelix
Copy link

templarfelix commented Apr 20, 2021

have date for solve this issue?

@danielvincenzi
Copy link

What is the estimated delivery of this issue?

@sandrogattuso
Copy link
Contributor

@dustinvanbuskirk is there any documentation on how this works from an API perspective? I had a look around the CRON triggers configuration and this is something that is not directly attached to the pipeline.
From what I've understood looking at the API calls made by the browser when editing the configuration, seems you have at least another 2 API endpoints involved in the generation of the CRON trigger.

Adding the feature in the resource_pipeline would go against the recommended practices https://www.terraform.io/docs/extend/hashicorp-provider-design-principles.html#resources-should-represent-a-single-api-object

I can help on the implementation of this but I would like to know if you had any thought around how this would look like from a resource perspective in terraform.

@kpurdon
Copy link

kpurdon commented Oct 20, 2021

@sandrogattuso any update here? I'm happy to help.

From an API perspective this is:

# create an event (returns the "event" which is a string id used in the next call)
curl  \
    -X POST \
    -H "Authorization: ${API_KEY}" \
    -H 'Content-Type: application/json; charset=utf-8' \
    -d '{"type":"cron","kind":"codefresh","secret":"!generate","values":{"expression":"0 0 1 1 1/1 ?","message":"some message"}}' \
    "https://g.codefresh.io/api/hermes/events?public=false"

# use the result as EVENT and attach it as a trigger to the pipeline
curl -X POST \
     -H "Authorization: ${API_KEY}" \
     -H 'Content-Type: application/json; charset=utf-8' \
     "https://g.codefresh.io/api/hermes/triggers/${EVENT}/${PIPELINE}"

from a Terraform resource perspective this would ideally be:

resource pipeline {
    trigger {
        // config here
    }

But as designed the trigger block is pretty overloaded. I could see either a separate trigger resource which takes a pipeline id, or a seperate trigger_cron block in the pipeline resource.

@kpurdon
Copy link

kpurdon commented Nov 3, 2021

@yaroslav-codefresh @denis-codefresh @pasha-codefresh (sorry; still unclear who maintainers are)

How can we move this forward; the main questions seem to be around design of the pipeline:trigger resource with the addition of CRON type triggers.

I (and possibly @sandrogattuso) have offered time to help implement; but I wouldn't want to dig in without design guidance from maintainers.

@denis-codefresh
Copy link
Collaborator

From my perspective it's better to embed trigger_cron resource in the pipeline (make a new block).
Event if API looks different for cron and git triggers it's better to keep them at same place in your "infrastructure code".
@ziv-codefresh WDYT?

@kpurdon
Copy link

kpurdon commented Dec 2, 2021

@denis-codefresh any update here? Can you make the call on a preferred design? If I were to put in the work for a trigger_cron would that be accepted?

@Laurent-OC
Copy link

@denis-codefresh any update here?

@expnch
Copy link
Contributor

expnch commented Apr 8, 2022

@denis-codefresh We are still waiting on an update.

@denis-codefresh
Copy link
Collaborator

@kpurdon Let's create a new trigger type block in addition to a regular trigger called cron_trigger.

@korenyoni
Copy link
Contributor

korenyoni commented Sep 14, 2022

Hi all. Sorry for the huge delay. We didn't really have a maintainer assigned to this repo (until now).

Basically, @denis-codefresh and I agreed that the git_trigger block should be deprecated, and standalone resources for each of our triggers should be supported:

  • codefresh_pipeline_git_trigger
  • codefresh_pipeline_cron_trigger
  • codefresh_pipeline_helm_trigger
  • codefresh_pipeline_registry_trigger

By the way, git triggers and the other 3 types of triggers use completely different APIs. Just something to note. The last 3 triggers in the list are based on something called hermes and thus use the hermes API in Codefresh.

There is some WIP stuff here in my fork https://github.com/korenyoni/terraform-provider-codefresh. We are also overhauling documentation, release automation, and probably a couple other small things we can fit into a minor release (0.1.0). We are expecting some turnaround on this in the next two weeks. My goal is to get this repo into better shape...

@korenyoni
Copy link
Contributor

We have a PR open for this now. What I said about the other 3 types of standalone triggers is actually going to be moved out to 0.2.0, even though it would have been ideal to release all 4 as part of 0.1.0 and deprecate the inline trigger.

korenyoni added a commit that referenced this issue Oct 3, 2022
## What

* Add resource for Pipeline Cron Trigger
* Fix Terraform Acceptance tests
* Add Acceptance Tests to CI
* Add release automation (release drafter, labeler, automatic changelog
drafting)
* GH Repo best practices (PULL_REQUEST_TEMPLATE, CODEOWNERS, etc)
* Misc documentation fixes

## Why

* Only git triggers are currently supported.
* Triggers should be a standalone resource. The next minor release
(`0.2.0`) will support remaining standalone triggers (registry, helm,
git).

## Notes

* Closes #39 
* Closes #88
* #84

Co-authored-by: Yonatan Koren <[email protected]>
Co-authored-by: korenyoni <[email protected]>
korenyoni added a commit that referenced this issue Oct 3, 2022
## What

* Fix changelog-from-release GHA

## Why

* Branch protection rules prevent this GHA from working, need
GITHUB_TOKEN corresponding to user with bypass

## Notes

#39
@korenyoni
Copy link
Contributor

korenyoni commented Aug 15, 2023

Hi all. Sorry for the huge delay. We didn't really have a maintainer assigned to this repo (until now).

Basically, @denis-codefresh and I agreed that the git_trigger block should be deprecated, and standalone resources for each of our triggers should be supported:

  • codefresh_pipeline_git_trigger
  • codefresh_pipeline_cron_trigger
  • codefresh_pipeline_helm_trigger
  • codefresh_pipeline_registry_trigger

By the way, git triggers and the other 3 types of triggers use completely different APIs. Just something to note. The last 3 triggers in the list are based on something called hermes and thus use the hermes API in Codefresh.

There is some WIP stuff here in my fork https://github.com/korenyoni/terraform-provider-codefresh. We are also overhauling documentation, release automation, and probably a couple other small things we can fit into a minor release (0.1.0). We are expecting some turnaround on this in the next two weeks. My goal is to get this repo into better shape...

Unfortunately we are going to be backtracking on the standalone *_trigger resources 😞. The reason being is that the Codefresh API previously used to create triggers (the Hermes API) is now being deprecated in favor of an expanded Pipelines API, and because Terraform resources should to map 1-1 with the API object they represent (a best practice also linked in this thread), this means that the triggers will have to be inline blocks.

This is going to be merged in #122 and initially done a pre-release, and then pipeline_cron_trigger will be deprecated and the documentation will be updated prior to 0.6.0 being officially released. Also, the expanded pipelines API is for now behind a feature flag and [email protected] needs to be contacted to expose it. The flipside is additional options such as variables and runtime environment in the inline cron trigger.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants