-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
feat: Add pattern matching support to atlantis plan/apply -d flag #2742
Conversation
thanks @jmediald-te for the contribution |
How is this different than --enable-regexp-cmd ? |
Thanks for your response @jamengual. I will try to work on the docs/tests later. @nitrocode - Based on the documentation, |
why we don't add patterns matching to the - d instead? we can reuse the
same flag used in - p to enable it.
what do you think?
…On Thu, Dec 1, 2022, 2:54 a.m. Josep Medialdea ***@***.***> wrote:
Thanks for your response @jamengual <https://github.com/jamengual>. I
will try to work on the docs/tests later.
@nitrocode <https://github.com/nitrocode> - Based on the documentation,
--enable-regexp-cmd allows us to use regular expressions when using the -p
command. It is often the case where we do not have all projects defined in
atlantis.yaml so the -p flag won't work. The atlantis apply/plan -f
<pattern> command would be equivalent to atlantis apply/plan -d <pattern>
if the -d flag had support for pattern matching.
—
Reply to this email directly, view it on GitHub
<#2742 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAQ3ERDQBB6Q3D676JUI3N3WLB7URANCNFSM6AAAAAASP7XH4U>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
Makes a lot of sense. It will be more consistent. I will work on that. |
@nitrocode I think this are two different use cases. This PR is used to define the runs based on the folder structure. Ignoring defined projects if I'm not mistaken. Sry I'm on my mobile so I can't check the code reasonable. My PR is about running all defined projects. |
@secustor as of today, you are technically correct about both prs, however the discussion has led back to a -d regexp. Both you and this pr author have attempted different solutions to the same problem. Since that is the case, i am suggesting that one of you drives the suggested solution using -d regexp and the other contributes as a pr reviewer (or author) so we can achieve the desired outcome 😄 |
Hey, sorry for my late reply. I have been very busy these days. I will be working on this today. What I am doing is basically adding support for filepath pattern matching into the If you want I can rename this PR to: feat: Add support for filepath pattern matching into atlantis plan/apply -d flag. Or create a new PR with the new name. |
I have just moved the functionality of the |
@@ -332,6 +342,15 @@ func (p *DefaultProjectCommandBuilder) buildPlanAllCommands(ctx *command.Context | |||
ctx.Log.Debug("moduleInfo for %s (matching %q) = %v", repoDir, p.AutoDetectModuleFiles, moduleInfo) | |||
modifiedProjects := p.ProjectFinder.DetermineProjects(ctx.Log, modifiedFiles, ctx.Pull.BaseRepo.FullName, repoDir, p.AutoplanFileList, moduleInfo) | |||
ctx.Log.Info("automatically determined that there were %d projects modified in this pull request: %s", len(modifiedProjects), modifiedProjects) | |||
|
|||
if p.EnableRegExpCmd && cmd.RepoRelDir != "" { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This code looks to be the same as lines 303 to 309.
Can we encapsulate this into a function to keep the code as DRY as possible?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or is there a better way to structure this so we can run this block for both if and else blocks?
@@ -482,6 +501,14 @@ func (p *DefaultProjectCommandBuilder) buildAllProjectCommands(ctx *command.Cont | |||
return nil, err | |||
} | |||
|
|||
if p.EnableRegExpCmd && commentCmd.RepoRelDir != "" { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Another duplicated block here
return filteredProjects, nil | ||
} | ||
|
||
func filterValidProjects(projects []valid.Project, filter string) ([]valid.Project, error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we combine both filterProjects
and filterValidProjects
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Only difference I see is models.Project
and proj.Path
vs valid.Project
and proj.Dir
return filteredProjects, nil | ||
} | ||
|
||
func filterPlans(plans []PendingPlan, filter string) ([]PendingPlan, error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This could also be combined with the above 2 functions. Only thing I see here that's different is using PendingPlan
and plan.RepoRelDir
@@ -112,6 +112,10 @@ func (c CommentCommand) IsForSpecificProject() bool { | |||
return c.RepoRelDir != "" || c.Workspace != "" || c.ProjectName != "" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please re-word the PR summary and title to reflect that this is no longer using the -f
flag and instead will add wildcard filtering using the existing -d
flag
func (c CommentCommand) HasDirPatternMatching() bool { | ||
return strings.Contains(c.RepoRelDir, "*") || strings.Contains(c.RepoRelDir, "?") || strings.Contains(c.RepoRelDir, "[") || strings.Contains(c.RepoRelDir, "]") | ||
} | ||
|
||
// CommandName returns the name of this command. | ||
func (c CommentCommand) CommandName() command.Name { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add tests for the new functionality
If we add a The following should work # should plan everything using the dev workspace
atlantis plan -d .* -w dev
# should plan everything using the prod workspace
atlantis plan -d .* -w prod
# should plan everything using the default workspace
atlantis plan -d .* |
@jmediald-te friendly ping here. We'd love to see this change in the next release if you have time. |
Hey @nitrocode, sorry for my late reply. I have been out of office during the holiday break. I will be working on this soon. |
Hi @jmediald-te . Any update on this? We'd love to see this feature. It's currently the second highest feature requested. :) |
This issue is stale because it has been open for 1 month with no activity. Remove stale label or comment or this will be closed in 1 month. |
This issue is stale because it has been open for 1 month with no activity. Remove stale label or comment or this will be closed in 1 month. |
if someone is willing to work on this and finish this pr , we will be happy to review |
This issue is stale because it has been open for 1 month with no activity. Remove stale label or comment or this will be closed in 1 month. |
@jmediald-te do you think you can continue this work? Thanks. |
This issue is stale because it has been open for 1 month with no activity. Remove stale label or comment or this will be closed in 1 month. |
Would love to see this make it to the finish line, any updates on when work can be resumed on it? |
The original committer has not responded in a while if you have time @jrodante and golango knowledge go for it. |
if anyone is interested, please reopen |
what
This PR adds support for filepath pattern matching expressions to the
-d/--dir
flag inatlantis plan/apply
commands. The user must use the--enable-regexp-cmd
server configuration flag to activate this feature.When we run
atlantis plan/apply -d <pattern>
, atlantis will behave exactly the same way as when runningatlantis plan/apply
. The only difference is that it will only plan/apply the projects whose relative dir matches with the specified pattern.why
references