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

fix: fixed validation pattern #47

Merged
merged 1 commit into from
Jan 5, 2025
Merged

fix: fixed validation pattern #47

merged 1 commit into from
Jan 5, 2025

Conversation

saidsef
Copy link
Owner

@saidsef saidsef commented Jan 5, 2025

No description provided.

@saidsef saidsef added the bug Something isn't working label Jan 5, 2025
@saidsef saidsef self-assigned this Jan 5, 2025
@saidsef
Copy link
Owner Author

saidsef commented Jan 5, 2025

/code pr

Copy link
Owner Author

@saidsef saidsef left a comment

Choose a reason for hiding this comment

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

🤖
The validation pattern change appears problematic:

  1. The regex ^[^h][^t][^t][^p][^s]?[^:] is overly complex and could reject valid URLs:
    • It requires 6 characters that aren't h,t,t,p,s,:
    • This would reject legitimate URLs that happen to start with those characters

Suggestion:

validation {
  condition     = length(var.url) > 0 && !startswith(var.url, "http://") && !startswith(var.url, "https://")
  error_message = "URL must not start with 'http://' or 'https://' and cannot be empty."
}

This would be:

  • More readable
  • More maintainable
  • More reliable at catching the intended cases
  • Properly validates the empty string case

> claude-3-5-sonnet-20241022 [pr | 2]

@saidsef saidsef force-pushed the fix-url-validation branch from 982ff8d to 1e4c68f Compare January 5, 2025 08:52
@saidsef saidsef merged commit df55b5a into main Jan 5, 2025
11 of 12 checks passed
@saidsef saidsef deleted the fix-url-validation branch January 5, 2025 08:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant