Skip to content

Add the ability to trigger the deployment on changes of the specified paths #1250

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.

Sign up for GitHub

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

Merged
pipecd-bot merged 3 commits into from
Dec 15, 2020
Merged

Add the ability to trigger the deployment on changes of the specified paths #1250

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
a23e416
Add ability to trigger the deployment on changes of the specified paths
nghialv Dec 15, 2020
f923c81
Fix name
nghialv Dec 15, 2020
cf78cb6
Update pkg/app/piped/trigger/trigger.go
nghialv Dec 15, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion docs/content/en/docs/user-guide/configuration-reference.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ spec:
| workloads | [][KubernetesWorkload](/docs/user-guide/configuration-reference/#kubernetesworkload) | Which Kubernetes resources should be considered as the Workloads of application. Empty means all Deployment resources. | No |
| trafficRouting | [KubernetesTrafficRouting](/docs/user-guide/configuration-reference/#kubernetestrafficrouting) | How to change traffic routing percentages. | No |
| sealedSecrets | [][SealedSecretMapping](/docs/user-guide/configuration-reference/#sealedsecretmapping) | The list of sealed secrets should be decrypted. | No |
<!-- | dependencies | []string | List of directories where their changes will trigger the deployment. | No | -->
| triggerPaths | []string | List of directories or files where their changes will trigger the deployment. Regular expression can be used. | No |

## Terraform application

Expand Down
1 change: 1 addition & 0 deletions pkg/app/piped/trigger/BUILD.bazel
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ go_library(
deps = [
"//pkg/app/api/service/pipedservice:go_default_library",
"//pkg/config:go_default_library",
"//pkg/filematcher:go_default_library",
"//pkg/git:go_default_library",
"//pkg/model:go_default_library",
"@com_github_google_uuid//:go_default_library",
Expand Down
14 changes: 0 additions & 14 deletions pkg/app/piped/trigger/deployment.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,14 +17,12 @@ package trigger
import (
"context"
"fmt"
"path/filepath"
"time"

"github.com/google/uuid"
"go.uber.org/zap"

"github.com/pipe-cd/pipe/pkg/app/api/service/pipedservice"
"github.com/pipe-cd/pipe/pkg/config"
"github.com/pipe-cd/pipe/pkg/git"
"github.com/pipe-cd/pipe/pkg/model"
)
Expand Down Expand Up @@ -72,18 +70,6 @@ func (t *Trigger) triggerDeployment(ctx context.Context, app *model.Application,
return
}

func (t *Trigger) loadDeploymentConfiguration(repoPath string, app *model.Application) (*config.Config, error) {
path := filepath.Join(repoPath, app.GitPath.GetDeploymentConfigFilePath())
cfg, err := config.LoadFromYAML(path)
if err != nil {
return nil, err
}
if appKind, ok := config.ToApplicationKind(cfg.Kind); !ok || appKind != app.Kind {
return nil, fmt.Errorf("application in deployment configuration file is not match, got: %s, expected: %s", appKind, app.Kind)
}
return cfg, nil
}

func (t *Trigger) reportMostRecentlyTriggeredDeployment(ctx context.Context, d *model.Deployment) error {
var (
err error
Expand Down
52 changes: 42 additions & 10 deletions pkg/app/piped/trigger/trigger.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ package trigger
import (
"context"
"fmt"
"path/filepath"
"strings"
"time"

Expand All @@ -30,6 +31,7 @@ import (

"github.com/pipe-cd/pipe/pkg/app/api/service/pipedservice"
"github.com/pipe-cd/pipe/pkg/config"
"github.com/pipe-cd/pipe/pkg/filematcher"
"github.com/pipe-cd/pipe/pkg/git"
"github.com/pipe-cd/pipe/pkg/model"
)
Expand Down Expand Up @@ -296,7 +298,17 @@ func (t *Trigger) checkApplication(ctx context.Context, app *model.Application,
if err != nil {
return err
}
if touched := isTouchedByChangedFiles(app.GitPath.Path, nil, changedFiles); !touched {

deployConfig, err := loadDeploymentConfiguration(repo.GetPath(), app)
if err != nil {
return err
}

touched, err := isTouchedByChangedFiles(app.GitPath.Path, deployConfig.TriggerPaths, changedFiles)
if err != nil {
return err
}
if !touched {
logger.Info("application was not touched by the new commit",
zap.String("most-recently-triggered-commit", preCommitHash),
)
Expand Down Expand Up @@ -386,7 +398,25 @@ func (t *Trigger) getMostRecentlyTriggeredDeployment(ctx context.Context, applic
return nil, err
}

func isTouchedByChangedFiles(appDir string, dependencyDirs []string, changedFiles []string) bool {
func loadDeploymentConfiguration(repoPath string, app *model.Application) (*config.GenericDeploymentSpec, error) {
path := filepath.Join(repoPath, app.GitPath.GetDeploymentConfigFilePath())
cfg, err := config.LoadFromYAML(path)
if err != nil {
return nil, err
}
if appKind, ok := config.ToApplicationKind(cfg.Kind); !ok || appKind != app.Kind {
return nil, fmt.Errorf("invalid application kind in the deployment config file, got: %s, expected: %s", appKind, app.Kind)
}

spec, ok := cfg.GetGenericDeployment()
if !ok {
return nil, fmt.Errorf("unsupported application kind: %s", app.Kind)
}

return &spec, nil
}

func isTouchedByChangedFiles(appDir string, changes []string, changedFiles []string) (bool, error) {
if !strings.HasSuffix(appDir, "/") {
appDir += "/"
}
Expand All @@ -395,19 +425,21 @@ func isTouchedByChangedFiles(appDir string, dependencyDirs []string, changedFile
// this application is considered as touched.
for _, cf := range changedFiles {
if ok := strings.HasPrefix(cf, appDir); ok {
return true
return true, nil
}
}

// If any files inside the app's dependencies was changed
// If any changed files matches the specified "changes"
// this application is consided as touched too.
for _, depDir := range dependencyDirs {
for _, cf := range changedFiles {
if ok := strings.HasPrefix(cf, depDir); ok {
return true
}
for _, change := range changes {
matcher, err := filematcher.NewPatternMatcher([]string{change})
if err != nil {
return false, err
}
if matcher.MatchesAny(changedFiles) {
return true, nil
Comment on lines +434 to +440
Copy link
Member

@nakabonne nakabonne Dec 15, 2020

Choose a reason for hiding this comment

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

Sorry if I'm dumb but is it needed to perform for loop? We just give all patterns to it, right?

		matcher, err := filematcher.NewPatternMatcher([]string{changes})
		if err != nil {
			return false, err
		}
		if matcher.MatchesAny(changedFiles) {

Copy link
Member Author

@nghialv nghialv Dec 15, 2020

Choose a reason for hiding this comment

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

We can give all patterns to filematcher and parse all of them before checking.
But by doing one by one we can have fast-return and reduce the number of unneeded regex parsing.
(In the future we can enable the regex cache too.)

Copy link
Member

@nakabonne nakabonne Dec 15, 2020

Choose a reason for hiding this comment

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

Ah, that makes sense! Okay, let's keep it as is.

}
}

return false
return false, nil
}
29 changes: 14 additions & 15 deletions pkg/app/piped/trigger/trigger_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,16 +22,15 @@ import (

func TestIsTouchedByChangedFiles(t *testing.T) {
testcases := []struct {
name string
appDir string
dependencyDirs []string
changedFiles []string
expected bool
name string
appDir string
changes []string
changedFiles []string
expected bool
}{
{
name: "not touched",
appDir: "app/demo",
dependencyDirs: nil,
name: "not touched",
appDir: "app/demo",
changedFiles: []string{
"app/hello.txt",
"app/foo/deployment.yaml",
Expand All @@ -47,21 +46,20 @@ func TestIsTouchedByChangedFiles(t *testing.T) {
expected: false,
},
{
name: "touched in app dir",
appDir: "app/demo",
dependencyDirs: nil,
name: "touched in app dir",
appDir: "app/demo",
changedFiles: []string{
"app/hello.txt",
"app/demo/deployment.yaml",
},
expected: true,
},
{
name: "touched in dependency dir",
name: "touched in the changes",
appDir: "app/demo",
dependencyDirs: []string{
changes: []string{
"charts/demo",
"charts/bar",
"charts/bar/*",
},
changedFiles: []string{
"app/hello.txt",
Expand All @@ -73,7 +71,8 @@ func TestIsTouchedByChangedFiles(t *testing.T) {

for _, tc := range testcases {
t.Run(tc.name, func(t *testing.T) {
got := isTouchedByChangedFiles(tc.appDir, tc.dependencyDirs, tc.changedFiles)
got, err := isTouchedByChangedFiles(tc.appDir, tc.changes, tc.changedFiles)
assert.NoError(t, err)
assert.Equal(t, tc.expected, got)
})
}
Expand Down
5 changes: 3 additions & 2 deletions pkg/config/deployment.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,9 @@ type GenericDeploymentSpec struct {
Pipeline *DeploymentPipeline `json:"pipeline"`
// The list of sealed secrets that should be decrypted.
SealedSecrets []SealedSecretMapping `json:"sealedSecrets"`
// List of directories where their changes will trigger the deployment.
Dependencies []string `json:"dependencies,omitempty"`
// List of directories or files where their changes will trigger the deployment.
// Regular expression can be used.
TriggerPaths []string `json:"triggerPaths,omitempty"`
}

func (s GenericDeploymentSpec) GetStage(index int32) (PipelineStage, bool) {
Expand Down