fix: cron validator regex escape characters #45
Conversation
|
Warning Rate limit exceeded@oycyc has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 13 minutes and 23 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (2)
WalkthroughThe pull request modifies the regex pattern in the Changes
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
main.tf(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
main.tf (1)
Pattern **/*.tf: You're a Terraform expert who has thoroughly studied all the documentation from Hashicorp https://developer.hashicorp.com/terraform/docs and OpenTofu https://opentofu.org/docs/.
You have a strong grasp of Terraform syntax and prioritize providing accurate and insightful code suggestions.
As a fan of the Cloud Posse / SweetOps ecosystem, you incorporate many of their best practices https://docs.cloudposse.com/best-practices/terraform/ while balancing them with general Terraform guidelines.
|
Coderabbit is suggesting the regex fix in the pic with the escape character. Could consider that. But I'm leaning more to just remove this validation in the first place. Terraform/Spacelift provider will error out anyways if not correct. |
gberenice
left a comment
gberenice
left a comment
There was a problem hiding this comment.
I think it's good to have this one. I agree that this is barely readable, but we can provide more examples on what is allowed by this precondition.
oycyc
changed the title
Updated to only address the whitespace. |
|
Some residue from |
| lifecycle { | ||
| precondition { | ||
| condition = alltrue([for schedule in try(local.stack_configs[each.key].drift_detection_schedule, var.drift_detection_schedule) : can(regex("^([0-9,\\-\\*]+\\s+){4}[0-9,\\-\\*]+$", schedule))]) | ||
| condition = alltrue([for schedule in try(local.stack_configs[each.key].drift_detection_schedule, var.drift_detection_schedule) : can(regex("^([0-9,\\-*/]+\\s+){4}[0-9,\\-*/]+$", schedule))]) |
There was a problem hiding this comment.
We should add a test to check that this works as expected with the / so we know this works as expected! Mind adding it?
There was a problem hiding this comment.
@oycyc let's get this merged without this test, but let's follow up with one!
There was a problem hiding this comment.
Sounds good, put thi s on my todo list
🤖 I have created a release *beep* *boop* --- ## [1.0.0](v0.7.0...v1.0.0) (2025-02-04) ### ⚠ BREAKING CHANGES * TF Version pinning has been updated to require terraform v1.9+ and OpenTofu v1.7+. Please update the consuming root module accordingly. (#43) * Scopes `tfvars` + `default_tf_workspace_enabled` settings under `automation_settings` object in StackConfig schema. [See README](https://github.com/masterpointio/terraform-spacelift-automation?tab=readme-ov-file#what-goes-in-a-stack-config-file-eg-stacksdevyaml-stackscommonyaml-stackyaml-and-similar) for example of StackConfig schema. (#42) ### Features * adds support for description as a templatestring ([#43](#43)) ([1bbb74f](1bbb74f)) * adds the runtime_overrides variable + tests ([#44](#44)) ([6030f94](6030f94)) * feat: allow specifying `space_name` that maps to space_id #46 * **schema:** adds initial JSON schema + StackConfig changes ([#42](#42)) ([f247b5e](f247b5e)) ### Bug Fixes * cron validator regex escape characters ([#45](#45)) ([81a386b](81a386b)) * dont clobber static config with overrides ([#50](#50)) ([b352674](b352674)) * external space changed so update test data ([#51](#51)) ([569d8d4](569d8d4)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). --------- Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Matt Gowie <[email protected]>
I'm doing a cron expression for
0 */6 * * *This condition is failing if cron expression contains
*/{number}.It's this escape character. Could fix it via the second statement where it's escaped.
But thinking that this condition is too strict, wondering if it can just be to check if it has 5 expressions.
Or if we even need this validator? since validating cron expressions looks messy.
Summary by CodeRabbit