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

🚀 Add workspace trigger patterns to CRD #349

Closed

Conversation

baptman21
Copy link
Contributor

Description

Add the fields FileTriggersEnabled, TriggerPatterns and TriggerPrefixes to the Workspace CRD to allow the user to manage those fields.

The fields are already present in the go-tfe package but are currently not editable from the CRD.

Usage Example

  • Deploy a workspace with the configuration
apiVersion: app.terraform.io/v1alpha2
kind: Workspace
metadata:
  name: tmp
spec:
  allowDestroyPlan: false
  applyMethod: auto
  description: "Example with Trigger Patterns"
  executionMode: agent
  name: example
  organization: padoa
  project:
    name: Default Project
  tags:
  - test
  terraformVariables:
    - hcl: false
      name: env
      sensitive: false
      value: dev
  token:
    secretKeyRef:
      key: token
      name: example-token
  versionControl:
    branch: main
    oAuthTokenID: ot-XXXXXXXXXXX
    repository: example
  triggerPatterns:
    - "example/test/*"
  fileTriggersEnabled: true
  workingDirectory: example/test

References

Community Note

  • Please do not leave "+1" or other comments that do not add relevant new information or questions, they generate extra noise for issue followers and do not help prioritize the request.
  • Please vote on this issue by adding a 👍 reaction to the original issue to help the community and maintainers prioritize this request.
  • If you are interested in working on this issue or have submitted a pull request, please leave a comment.

@baptman21 baptman21 requested a review from a team as a code owner February 28, 2024 09:34
@hashicorp-cla
Copy link

hashicorp-cla commented Feb 28, 2024

CLA assistant check
All committers have signed the CLA.

@arybolovlev
Copy link
Contributor

Hi @baptman21,

Thank you for your interest in contributing to this project. I am going to review this PR next week and come back to you with my comments.

Thank you.

@baptman21 baptman21 force-pushed the feat-workspace-trigger-patterns branch from f8f930a to f0b9660 Compare March 5, 2024 07:27
@arybolovlev
Copy link
Contributor

Hi @baptman21,

Thank you again for your interest in contributing to this project.

There are a few things that we need to work on here and I would be happy to assist you with them. Please let me know if you prefer to get a full list or we can go step by step.

To set the right expectation, we won't be able to include your changes in the nearest release since it will happen soon and we have already settled with the list of the changes that will be included in it.

We can start with Workspace CR validation. It aims to catch possible mistakes in the custom resources. Here are two that we can begin with:

  • Only one of TriggerPatterns or TriggerPrefixes is allowed since they would conflict with each other.
  • TriggerPrefixes requires non-empty WorkingDirectory.

Please, update unit tests for validations that you will add. You should be able to run them without accessing TFC. Here you can read how to do that.

Please, let me know if you need assistance.

Thanks!

@baptman21
Copy link
Contributor Author

Thanks for the update. I wasn't sure whether we needed to check here the validity of the settings or let the TF cloud API throw the error. I will make the changes as you requested.

@baptman21
Copy link
Contributor Author

Hello @arybolovlev,

I added the validation for those 3 cases:

  • Only one of TriggerPatterns or TriggerPrefixes can be defined
  • TriggerPrefixes required non-empty WorkingDirectory
  • TriggerPatterns or TriggerPrefixes require FileTriggerEnabled set to true

The last one is not a specification of the API but rather a way to avoid mistakes like forgetting to enable the FileTriggerEnabled but defining the patterns, what do you think ?

Let me know if there are any other changes to make.

@arybolovlev
Copy link
Contributor

Hi @baptman21,

I want to let you know that I remember about this PR and will come back once we cut a new release. Thank you for your patience and great job!

@baptman21
Copy link
Contributor Author

Hello, for clarity I moved my code to a different fork, and updated the code to resolve the conflicts from the latest versions of the operator. The new PR is now #496. Closing this one in the meantime.

@baptman21 baptman21 closed this Oct 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

🤔 Choose when runs should be triggered by VCS changes.
3 participants