From c76b93310e2f4d719361d9b7cd8406bcef91c140 Mon Sep 17 00:00:00 2001 From: daemon1024 Date: Mon, 15 Mar 2021 20:05:52 +0530 Subject: [PATCH 01/15] initial remove-members tool implementation in go --- hack/remove-members.go | 123 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 123 insertions(+) create mode 100644 hack/remove-members.go diff --git a/hack/remove-members.go b/hack/remove-members.go new file mode 100644 index 0000000000..afa47d181d --- /dev/null +++ b/hack/remove-members.go @@ -0,0 +1,123 @@ +package main + +import ( + "bufio" + "flag" + "fmt" + "io/ioutil" + "log" + "os" + "path/filepath" + "regexp" +) + +var dryrun bool +var repoRoot string + +func main() { + flag.StringVar(&repoRoot, "root", ".", "Root of the repository") + flag.BoolVar(&dryrun, "dryrun", true, "Enable Dryrun or not") + + flag.Parse() + + memberList, err := readMemberList(flag.Args()[0]) + if err != nil { + log.Fatal(err) + } + + configPath := repoRoot + "/config" + + err = removeMembers(memberList, configPath) + if err != nil { + log.Fatal(err) + } + +} + +func readMemberList(path string) ([]string, error) { + file, err := os.Open(path) + if err != nil { + return nil, err + } + defer file.Close() + + var members []string + scanner := bufio.NewScanner(file) + for scanner.Scan() { + members = append(members, scanner.Text()) + } + return members, scanner.Err() +} + +func removeMembers(memberList []string, configPath string) error { + + var orgs, teams []string + + for _, member := range memberList { + count := 0 + fmt.Print(member) + + err := filepath.Walk(configPath, func(path string, info os.FileInfo, err error) error { + if err != nil { + return err + } + if info.IsDir() { + return nil + } + if matched, err := filepath.Match("*.yaml", filepath.Base(path)); err != nil { + return err + } else if matched { + removed, err := removeMemberFromFile(member, path) + if err != nil { + return err + } + + if removed { + count++ + if info.Name() == "org.yaml" { + orgs = append(orgs, path) + } + if info.Name() == "teams.yaml" { + teams = append(teams, path) + } + } + } + return nil + }) + if err != nil { + return err + } + fmt.Print("\n", count) + } + + return nil +} + +func removeMemberFromFile(member string, path string) (bool, error) { + + content, err := ioutil.ReadFile(path) + if err != nil { + return false, err + } + + re := regexp.MustCompile(`(\s+)?- ` + member + `(.*)?`) + + if re.Match(content) { + + if dryrun == true { + return true, nil + } + + updatedContent := re.ReplaceAll(content, []byte("")) + err = ioutil.WriteFile(path, updatedContent, 0666) + + if err != nil { + return false, err + } + + return true, nil + } + + return false, nil + +} From 323261c60599f9491fed43eeb95dc0986516e2c8 Mon Sep 17 00:00:00 2001 From: daemon1024 Date: Mon, 15 Mar 2021 21:53:38 +0530 Subject: [PATCH 02/15] execute git commands when not dry run --- hack/remove-members.go | 61 +++++++++++++++++++++++++++++++++++++----- 1 file changed, 55 insertions(+), 6 deletions(-) diff --git a/hack/remove-members.go b/hack/remove-members.go index afa47d181d..fce376c010 100644 --- a/hack/remove-members.go +++ b/hack/remove-members.go @@ -7,8 +7,11 @@ import ( "io/ioutil" "log" "os" + "os/exec" "path/filepath" "regexp" + "sort" + "strings" ) var dryrun bool @@ -50,10 +53,8 @@ func readMemberList(path string) ([]string, error) { } func removeMembers(memberList []string, configPath string) error { - - var orgs, teams []string - for _, member := range memberList { + var orgs, teams []string count := 0 fmt.Print(member) @@ -75,10 +76,10 @@ func removeMembers(memberList []string, configPath string) error { if removed { count++ if info.Name() == "org.yaml" { - orgs = append(orgs, path) + orgs = append(orgs, filepath.Base(filepath.Dir(path))) } if info.Name() == "teams.yaml" { - teams = append(teams, path) + teams = append(teams, filepath.Base(filepath.Dir(path))) } } } @@ -87,7 +88,15 @@ func removeMembers(memberList []string, configPath string) error { if err != nil { return err } - fmt.Print("\n", count) + + sort.Strings(orgs) + sort.Strings(teams) + + fmt.Printf("\n Orgs: %v\n Teams: %v\n Number of occurences: %d\n", orgs, teams, count) + + if count > 0 { + commitRemovedMembers(member, orgs, teams) + } } return nil @@ -115,9 +124,49 @@ func removeMemberFromFile(member string, path string) (bool, error) { return false, err } + cmd := exec.Command("git", "add", path) + err := cmd.Run() + + if err != nil { + log.Fatal(err) + } + return true, nil } return false, nil } + +func commitRemovedMembers(member string, orgs []string, teams []string) { + cmd := []string{"commit"} + + orgCommitMsg := "Remove " + member + " from the " + if len(orgs) == 1 { + orgCommitMsg += orgs[0] + " org" + cmd = append(cmd, "-m", orgCommitMsg) + } else if len(orgs) >= 1 { + orgCommitMsg += strings.Join(orgs, ", ") + " orgs" + cmd = append(cmd, "-m", orgCommitMsg) + } + + teamCommitMsg := "Remove " + member + " from " + if len(teams) == 1 { + teamCommitMsg += teams[0] + " team" + cmd = append(cmd, "-m", teamCommitMsg) + } else if len(teams) >= 1 { + teamCommitMsg += strings.Join(teams, ", ") + " teams" + cmd = append(cmd, "-m", teamCommitMsg) + } + + fmt.Printf("\nCommit Command: %q\n\n", strings.Join(cmd, " ")) + + if !dryrun { + cmd := exec.Command("git", cmd...) + err := cmd.Run() + + if err != nil { + log.Fatal(err) + } + } +} From 1dab05fc76b5d4f4aa0778a42af565043448e0e7 Mon Sep 17 00:00:00 2001 From: daemon1024 Date: Mon, 15 Mar 2021 22:42:42 +0530 Subject: [PATCH 03/15] document the tool --- hack/remove-members.go | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/hack/remove-members.go b/hack/remove-members.go index fce376c010..08deb0e7ad 100644 --- a/hack/remove-members.go +++ b/hack/remove-members.go @@ -37,6 +37,7 @@ func main() { } +//readMemberList reads the list of members to be removed from the given filepath func readMemberList(path string) ([]string, error) { file, err := os.Open(path) if err != nil { @@ -52,6 +53,7 @@ func readMemberList(path string) ([]string, error) { return members, scanner.Err() } +//removeMembers walks through the config directory and removes the occurences of the given member name func removeMembers(memberList []string, configPath string) error { for _, member := range memberList { var orgs, teams []string @@ -65,6 +67,7 @@ func removeMembers(memberList []string, configPath string) error { if info.IsDir() { return nil } + if matched, err := filepath.Match("*.yaml", filepath.Base(path)); err != nil { return err } else if matched { @@ -73,6 +76,7 @@ func removeMembers(memberList []string, configPath string) error { return err } + //Record the org/team name when a member is removed from it if removed { count++ if info.Name() == "org.yaml" { @@ -94,6 +98,7 @@ func removeMembers(memberList []string, configPath string) error { fmt.Printf("\n Orgs: %v\n Teams: %v\n Number of occurences: %d\n", orgs, teams, count) + //Proceed to committing changes if member is actually removed from somewhere if count > 0 { commitRemovedMembers(member, orgs, teams) } @@ -113,6 +118,7 @@ func removeMemberFromFile(member string, path string) (bool, error) { if re.Match(content) { + //Mofify the file only if it's not a dry run if dryrun == true { return true, nil } @@ -161,6 +167,7 @@ func commitRemovedMembers(member string, orgs []string, teams []string) { fmt.Printf("\nCommit Command: %q\n\n", strings.Join(cmd, " ")) + //Execute the git command only if not a dry run if !dryrun { cmd := exec.Command("git", cmd...) err := cmd.Run() From 9a1daf26a7838ccb504a8d7b25795f87a9018b6c Mon Sep 17 00:00:00 2001 From: daemon1024 Date: Mon, 15 Mar 2021 22:54:14 +0530 Subject: [PATCH 04/15] Provide usage instructions if no args provided --- hack/remove-members.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/hack/remove-members.go b/hack/remove-members.go index 08deb0e7ad..180d93683e 100644 --- a/hack/remove-members.go +++ b/hack/remove-members.go @@ -23,6 +23,11 @@ func main() { flag.Parse() + if len(flag.Args()) < 1 { + fmt.Print("Usage: remove-members [flags] \n\nFlags:\n\t-root\tstring\tpath to root directory of the repository\n\t-dryrun\tboolean\tEnable/Disable dryrun (Default: Enabled)") + os.Exit(0) + } + memberList, err := readMemberList(flag.Args()[0]) if err != nil { log.Fatal(err) From c9a68101687231dc5e0977c179337472f3bcbb15 Mon Sep 17 00:00:00 2001 From: daemon1024 Date: Tue, 16 Mar 2021 15:20:19 +0530 Subject: [PATCH 05/15] combine iferr logs with function calls --- hack/remove-members.go | 20 ++++++-------------- 1 file changed, 6 insertions(+), 14 deletions(-) diff --git a/hack/remove-members.go b/hack/remove-members.go index 180d93683e..28c8b83674 100644 --- a/hack/remove-members.go +++ b/hack/remove-members.go @@ -35,8 +35,7 @@ func main() { configPath := repoRoot + "/config" - err = removeMembers(memberList, configPath) - if err != nil { + if err = removeMembers(memberList, configPath); err != nil { log.Fatal(err) } @@ -65,7 +64,7 @@ func removeMembers(memberList []string, configPath string) error { count := 0 fmt.Print(member) - err := filepath.Walk(configPath, func(path string, info os.FileInfo, err error) error { + if err := filepath.Walk(configPath, func(path string, info os.FileInfo, err error) error { if err != nil { return err } @@ -93,8 +92,7 @@ func removeMembers(memberList []string, configPath string) error { } } return nil - }) - if err != nil { + }); err != nil { return err } @@ -129,16 +127,12 @@ func removeMemberFromFile(member string, path string) (bool, error) { } updatedContent := re.ReplaceAll(content, []byte("")) - err = ioutil.WriteFile(path, updatedContent, 0666) - - if err != nil { + if err = ioutil.WriteFile(path, updatedContent, 0666); err != nil { return false, err } cmd := exec.Command("git", "add", path) - err := cmd.Run() - - if err != nil { + if err := cmd.Run(); err != nil { log.Fatal(err) } @@ -175,9 +169,7 @@ func commitRemovedMembers(member string, orgs []string, teams []string) { //Execute the git command only if not a dry run if !dryrun { cmd := exec.Command("git", cmd...) - err := cmd.Run() - - if err != nil { + if err := cmd.Run(); err != nil { log.Fatal(err) } } From 73f657a96ff7948effa496523ef9c8042a87a577 Mon Sep 17 00:00:00 2001 From: daemon1024 Date: Tue, 16 Mar 2021 15:22:02 +0530 Subject: [PATCH 06/15] update flags and usage instructions --- hack/remove-members.go | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/hack/remove-members.go b/hack/remove-members.go index 28c8b83674..d3acbea5ab 100644 --- a/hack/remove-members.go +++ b/hack/remove-members.go @@ -15,16 +15,21 @@ import ( ) var dryrun bool -var repoRoot string +var configPath string func main() { - flag.StringVar(&repoRoot, "root", ".", "Root of the repository") - flag.BoolVar(&dryrun, "dryrun", true, "Enable Dryrun or not") + flag.StringVar(&configPath, "path", ".", "Path to config directory/subdirectory") + flag.BoolVar(&dryrun, "dryrun", true, "Enable Dryrun or not. This flag controls whether the changes are simulated or live. If the changes are simulated it will only print the removal details and the commit message") flag.Parse() - if len(flag.Args()) < 1 { - fmt.Print("Usage: remove-members [flags] \n\nFlags:\n\t-root\tstring\tpath to root directory of the repository\n\t-dryrun\tboolean\tEnable/Disable dryrun (Default: Enabled)") + flag.Usage = func() { + fmt.Fprintf(os.Stderr, "Usage: remove-members [flags] \n") + flag.PrintDefaults() + } + + if len(flag.Args()) != 1 { + flag.Usage() os.Exit(0) } @@ -33,8 +38,6 @@ func main() { log.Fatal(err) } - configPath := repoRoot + "/config" - if err = removeMembers(memberList, configPath); err != nil { log.Fatal(err) } From 6668a0fe45a6fb1d36201d66f351780f5757f622 Mon Sep 17 00:00:00 2001 From: daemon1024 Date: Wed, 17 Mar 2021 15:50:31 +0530 Subject: [PATCH 07/15] update remove-members.go * use pflags * use iouyil instead of bufio for reading member list * preserve filemode after removals --- hack/remove-members.go | 23 +++++++++-------------- 1 file changed, 9 insertions(+), 14 deletions(-) diff --git a/hack/remove-members.go b/hack/remove-members.go index d3acbea5ab..66086e9912 100644 --- a/hack/remove-members.go +++ b/hack/remove-members.go @@ -1,8 +1,6 @@ package main import ( - "bufio" - "flag" "fmt" "io/ioutil" "log" @@ -12,6 +10,8 @@ import ( "regexp" "sort" "strings" + + flag "github.com/spf13/pflag" ) var dryrun bool @@ -46,18 +46,13 @@ func main() { //readMemberList reads the list of members to be removed from the given filepath func readMemberList(path string) ([]string, error) { - file, err := os.Open(path) + file, err := ioutil.ReadFile(path) if err != nil { return nil, err } - defer file.Close() - var members []string - scanner := bufio.NewScanner(file) - for scanner.Scan() { - members = append(members, scanner.Text()) - } - return members, scanner.Err() + var members = strings.Split(string(file), "\n") + return members, nil } //removeMembers walks through the config directory and removes the occurences of the given member name @@ -78,7 +73,7 @@ func removeMembers(memberList []string, configPath string) error { if matched, err := filepath.Match("*.yaml", filepath.Base(path)); err != nil { return err } else if matched { - removed, err := removeMemberFromFile(member, path) + removed, err := removeMemberFromFile(member, path, info) if err != nil { return err } @@ -113,7 +108,7 @@ func removeMembers(memberList []string, configPath string) error { return nil } -func removeMemberFromFile(member string, path string) (bool, error) { +func removeMemberFromFile(member string, path string, info os.FileInfo) (bool, error) { content, err := ioutil.ReadFile(path) if err != nil { @@ -125,12 +120,12 @@ func removeMemberFromFile(member string, path string) (bool, error) { if re.Match(content) { //Mofify the file only if it's not a dry run - if dryrun == true { + if dryrun { return true, nil } updatedContent := re.ReplaceAll(content, []byte("")) - if err = ioutil.WriteFile(path, updatedContent, 0666); err != nil { + if err = ioutil.WriteFile(path, updatedContent, info.Mode()); err != nil { return false, err } From 9c6802983c28e67105310dcab4444b71c60af856 Mon Sep 17 00:00:00 2001 From: daemon1024 Date: Wed, 17 Mar 2021 15:53:26 +0530 Subject: [PATCH 08/15] update usage details --- hack/remove-members.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/hack/remove-members.go b/hack/remove-members.go index 66086e9912..497c0fbcfc 100644 --- a/hack/remove-members.go +++ b/hack/remove-members.go @@ -19,12 +19,12 @@ var configPath string func main() { flag.StringVar(&configPath, "path", ".", "Path to config directory/subdirectory") - flag.BoolVar(&dryrun, "dryrun", true, "Enable Dryrun or not. This flag controls whether the changes are simulated or live. If the changes are simulated it will only print the removal details and the commit message") + flag.BoolVar(&dryrun, "dryrun", true, "Enable Dryrun or not. Dryrun simulates changes to be applied and prints removal details") flag.Parse() flag.Usage = func() { - fmt.Fprintf(os.Stderr, "Usage: remove-members [flags] \n") + fmt.Fprintf(os.Stderr, "Usage: remove-members [--dryrun] [--path] member-file (file-containing-members-list)\n") flag.PrintDefaults() } From b38877a60e7f15e3308f779e5d5fd86230eecc23 Mon Sep 17 00:00:00 2001 From: daemon1024 Date: Wed, 17 Mar 2021 22:27:11 +0530 Subject: [PATCH 09/15] refactor remove-members tool in cmd/remove-members --- hack/remove-members.go => cmd/remove-members/main.go | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename hack/remove-members.go => cmd/remove-members/main.go (100%) diff --git a/hack/remove-members.go b/cmd/remove-members/main.go similarity index 100% rename from hack/remove-members.go rename to cmd/remove-members/main.go From 3fe1a196577c72334d9fde749d6d0030c0ffeff7 Mon Sep 17 00:00:00 2001 From: daemon1024 Date: Wed, 17 Mar 2021 22:28:20 +0530 Subject: [PATCH 10/15] update go.mod with new deps --- go.mod | 1 + go.sum | 1 + 2 files changed, 2 insertions(+) diff --git a/go.mod b/go.mod index 04a7439032..78d8229459 100644 --- a/go.mod +++ b/go.mod @@ -5,6 +5,7 @@ go 1.13 require ( github.com/ghodss/yaml v1.0.0 github.com/sirupsen/logrus v1.4.2 + github.com/spf13/pflag v1.0.5 k8s.io/apimachinery v0.0.0-20190817020851-f2f3a405f61d k8s.io/test-infra v0.0.0-20191222193732-de81526abe72 ) diff --git a/go.sum b/go.sum index 3e3a312c3d..d8c5bd6c67 100644 --- a/go.sum +++ b/go.sum @@ -316,6 +316,7 @@ github.com/spf13/jwalterweatherman v1.0.0/go.mod h1:cQK4TGJAtQXfYWX+Ddv3mKDzgVb6 github.com/spf13/pflag v1.0.1/go.mod h1:DYY7MBk1bdzusC3SYhjObp+wFpr4gzcvqqNjLnInEg4= github.com/spf13/pflag v1.0.2/go.mod h1:DYY7MBk1bdzusC3SYhjObp+wFpr4gzcvqqNjLnInEg4= github.com/spf13/pflag v1.0.3/go.mod h1:DYY7MBk1bdzusC3SYhjObp+wFpr4gzcvqqNjLnInEg4= +github.com/spf13/pflag v1.0.5 h1:iy+VFUOCP1a+8yFto/drg2CJ5u0yRoB7fZw3DKv/JXA= github.com/spf13/pflag v1.0.5/go.mod h1:McXfInJRrz4CZXVZOBLb0bTZqETkiAhM9Iw0y3An2Bg= github.com/spf13/viper v1.3.2/go.mod h1:ZiWeW+zYFKm7srdB9IoDzzZXaJaI5eL9QjNiN/DMA2s= github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= From 4e46ddb5f834ac5c151ca8bd2c40245275f9ac0f Mon Sep 17 00:00:00 2001 From: daemon1024 Date: Wed, 17 Mar 2021 22:54:36 +0530 Subject: [PATCH 11/15] add tests and test files --- cmd/remove-members/main.go | 2 +- cmd/remove-members/main_test.go | 28 ++++++++++++++++++++++++++++ cmd/remove-members/test/members.txt | 2 ++ cmd/remove-members/test/org.yaml | 3 +++ cmd/remove-members/test/teams.yaml | 15 +++++++++++++++ 5 files changed, 49 insertions(+), 1 deletion(-) create mode 100644 cmd/remove-members/main_test.go create mode 100644 cmd/remove-members/test/members.txt create mode 100644 cmd/remove-members/test/org.yaml create mode 100644 cmd/remove-members/test/teams.yaml diff --git a/cmd/remove-members/main.go b/cmd/remove-members/main.go index 497c0fbcfc..dd15a3f61d 100644 --- a/cmd/remove-members/main.go +++ b/cmd/remove-members/main.go @@ -14,7 +14,7 @@ import ( flag "github.com/spf13/pflag" ) -var dryrun bool +var dryrun bool = true var configPath string func main() { diff --git a/cmd/remove-members/main_test.go b/cmd/remove-members/main_test.go new file mode 100644 index 0000000000..40051d7750 --- /dev/null +++ b/cmd/remove-members/main_test.go @@ -0,0 +1,28 @@ +package main + +import ( + "reflect" + "testing" +) + +func TestReadMemberList(t *testing.T) { + memberList, err := readMemberList("test/members.txt") + if err != nil { + t.Error(err) + } + + expectedMemberList := []string{"johndoe", "janedoe"} + + if !reflect.DeepEqual(memberList, expectedMemberList) { + t.Errorf("Values mismatch. Expected: %v Actual: %v", expectedMemberList, memberList) + } +} + +func TestRemoveMembers(t *testing.T) { + + memberList := []string{"johndoe", "janedoe"} + + if err := removeMembers(memberList, "test"); err != nil { + t.Error(err) + } +} diff --git a/cmd/remove-members/test/members.txt b/cmd/remove-members/test/members.txt new file mode 100644 index 0000000000..3e779f5785 --- /dev/null +++ b/cmd/remove-members/test/members.txt @@ -0,0 +1,2 @@ +johndoe +janedoe \ No newline at end of file diff --git a/cmd/remove-members/test/org.yaml b/cmd/remove-members/test/org.yaml new file mode 100644 index 0000000000..1e64c214fa --- /dev/null +++ b/cmd/remove-members/test/org.yaml @@ -0,0 +1,3 @@ +members: + - loremipsum + - foobar \ No newline at end of file diff --git a/cmd/remove-members/test/teams.yaml b/cmd/remove-members/test/teams.yaml new file mode 100644 index 0000000000..3126647686 --- /dev/null +++ b/cmd/remove-members/test/teams.yaml @@ -0,0 +1,15 @@ +teams: + test-team: + description: "" + members: + - loremipsum + - johndoe #some random comment + - foobar #some random comment 2 + - janedoe + test-team-two: + description: "" + members: + - loremipsum + - johndoe + - foobar + From 7c23c349aacbdbc1667fcc3b34d4df744092a96e Mon Sep 17 00:00:00 2001 From: daemon1024 Date: Wed, 17 Mar 2021 23:07:02 +0530 Subject: [PATCH 12/15] update count metric to be more accurate --- cmd/remove-members/main.go | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/cmd/remove-members/main.go b/cmd/remove-members/main.go index dd15a3f61d..338a93c63f 100644 --- a/cmd/remove-members/main.go +++ b/cmd/remove-members/main.go @@ -73,14 +73,14 @@ func removeMembers(memberList []string, configPath string) error { if matched, err := filepath.Match("*.yaml", filepath.Base(path)); err != nil { return err } else if matched { - removed, err := removeMemberFromFile(member, path, info) + removed, removeCount, err := removeMemberFromFile(member, path, info) if err != nil { return err } //Record the org/team name when a member is removed from it if removed { - count++ + count += removeCount if info.Name() == "org.yaml" { orgs = append(orgs, filepath.Base(filepath.Dir(path))) } @@ -108,25 +108,27 @@ func removeMembers(memberList []string, configPath string) error { return nil } -func removeMemberFromFile(member string, path string, info os.FileInfo) (bool, error) { +func removeMemberFromFile(member string, path string, info os.FileInfo) (bool, int, error) { content, err := ioutil.ReadFile(path) if err != nil { - return false, err + return false, 0, err } re := regexp.MustCompile(`(\s+)?- ` + member + `(.*)?`) - if re.Match(content) { + matches := re.FindAllIndex(content, -1) + + if len(matches) >= 1 { //Mofify the file only if it's not a dry run if dryrun { - return true, nil + return true, len(matches), nil } updatedContent := re.ReplaceAll(content, []byte("")) if err = ioutil.WriteFile(path, updatedContent, info.Mode()); err != nil { - return false, err + return false, len(matches), err } cmd := exec.Command("git", "add", path) @@ -134,10 +136,10 @@ func removeMemberFromFile(member string, path string, info os.FileInfo) (bool, e log.Fatal(err) } - return true, nil + return true, len(matches), nil } - return false, nil + return false, len(matches), nil } From 8d352b6a190cac42016e83d35f5443757e04c55e Mon Sep 17 00:00:00 2001 From: daemon1024 Date: Tue, 23 Mar 2021 16:49:48 +0530 Subject: [PATCH 13/15] update remove-members/main.go * standardize flags and use struct to parse them * exit with 1 on error * match yaml files using Ext() * update logic for generating executable command --- cmd/remove-members/main.go | 104 ++++++++++++++++++++----------------- 1 file changed, 55 insertions(+), 49 deletions(-) diff --git a/cmd/remove-members/main.go b/cmd/remove-members/main.go index 338a93c63f..33593fe037 100644 --- a/cmd/remove-members/main.go +++ b/cmd/remove-members/main.go @@ -14,31 +14,41 @@ import ( flag "github.com/spf13/pflag" ) -var dryrun bool = true -var configPath string +type options struct { + confirm bool + configPath string +} -func main() { - flag.StringVar(&configPath, "path", ".", "Path to config directory/subdirectory") - flag.BoolVar(&dryrun, "dryrun", true, "Enable Dryrun or not. Dryrun simulates changes to be applied and prints removal details") +func parseOptions() options { + var o options + flag.StringVar(&o.configPath, "path", ".", "Path to config directory/subdirectory") + flag.BoolVar(&o.confirm, "confirm", true, "Modify the actual files or just simulate changes") flag.Parse() flag.Usage = func() { - fmt.Fprintf(os.Stderr, "Usage: remove-members [--dryrun] [--path] member-file (file-containing-members-list)\n") + fmt.Fprintf(os.Stderr, "Usage: remove-members [--confirm] [--path] member-file (file-containing-members-list)\n") flag.PrintDefaults() } if len(flag.Args()) != 1 { flag.Usage() - os.Exit(0) + os.Exit(1) } + return o +} + +func main() { + + o := parseOptions() + memberList, err := readMemberList(flag.Args()[0]) if err != nil { log.Fatal(err) } - if err = removeMembers(memberList, configPath); err != nil { + if err = removeMembers(memberList, o); err != nil { log.Fatal(err) } @@ -56,13 +66,13 @@ func readMemberList(path string) ([]string, error) { } //removeMembers walks through the config directory and removes the occurences of the given member name -func removeMembers(memberList []string, configPath string) error { +func removeMembers(memberList []string, o options) error { for _, member := range memberList { var orgs, teams []string count := 0 fmt.Print(member) - if err := filepath.Walk(configPath, func(path string, info os.FileInfo, err error) error { + if err := filepath.Walk(o.configPath, func(path string, info os.FileInfo, err error) error { if err != nil { return err } @@ -70,25 +80,25 @@ func removeMembers(memberList []string, configPath string) error { return nil } - if matched, err := filepath.Match("*.yaml", filepath.Base(path)); err != nil { + if filepath.Ext(path) != ".yaml" { + return nil + } + removed, removeCount, err := removeMemberFromFile(member, path, info, o.confirm) + if err != nil { return err - } else if matched { - removed, removeCount, err := removeMemberFromFile(member, path, info) - if err != nil { - return err - } + } - //Record the org/team name when a member is removed from it - if removed { - count += removeCount - if info.Name() == "org.yaml" { - orgs = append(orgs, filepath.Base(filepath.Dir(path))) - } - if info.Name() == "teams.yaml" { - teams = append(teams, filepath.Base(filepath.Dir(path))) - } + //Record the org/team name when a member is removed from it + if removed { + count += removeCount + if info.Name() == "org.yaml" { + orgs = append(orgs, filepath.Base(filepath.Dir(path))) + } + if info.Name() == "teams.yaml" { + teams = append(teams, filepath.Base(filepath.Dir(path))) } } + return nil }); err != nil { return err @@ -101,14 +111,14 @@ func removeMembers(memberList []string, configPath string) error { //Proceed to committing changes if member is actually removed from somewhere if count > 0 { - commitRemovedMembers(member, orgs, teams) + commitRemovedMembers(member, orgs, teams, o.confirm) } } return nil } -func removeMemberFromFile(member string, path string, info os.FileInfo) (bool, int, error) { +func removeMemberFromFile(member string, path string, info os.FileInfo, confirm bool) (bool, int, error) { content, err := ioutil.ReadFile(path) if err != nil { @@ -121,19 +131,16 @@ func removeMemberFromFile(member string, path string, info os.FileInfo) (bool, i if len(matches) >= 1 { - //Mofify the file only if it's not a dry run - if dryrun { - return true, len(matches), nil - } - - updatedContent := re.ReplaceAll(content, []byte("")) - if err = ioutil.WriteFile(path, updatedContent, info.Mode()); err != nil { - return false, len(matches), err - } + if confirm { + updatedContent := re.ReplaceAll(content, []byte("")) + if err = ioutil.WriteFile(path, updatedContent, info.Mode()); err != nil { + return false, len(matches), err + } - cmd := exec.Command("git", "add", path) - if err := cmd.Run(); err != nil { - log.Fatal(err) + cmd := exec.Command("git", "add", path) + if err := cmd.Run(); err != nil { + log.Fatal(err) + } } return true, len(matches), nil @@ -143,8 +150,11 @@ func removeMemberFromFile(member string, path string, info os.FileInfo) (bool, i } -func commitRemovedMembers(member string, orgs []string, teams []string) { - cmd := []string{"commit"} +func commitRemovedMembers(member string, orgs []string, teams []string, confirm bool) { + cmd := []string{"echo", "git", "commit"} + if confirm { + cmd = cmd[1:] + } orgCommitMsg := "Remove " + member + " from the " if len(orgs) == 1 { @@ -164,13 +174,9 @@ func commitRemovedMembers(member string, orgs []string, teams []string) { cmd = append(cmd, "-m", teamCommitMsg) } - fmt.Printf("\nCommit Command: %q\n\n", strings.Join(cmd, " ")) - - //Execute the git command only if not a dry run - if !dryrun { - cmd := exec.Command("git", cmd...) - if err := cmd.Run(); err != nil { - log.Fatal(err) - } + e := exec.Command(cmd[0], cmd[1:]...) + if err := e.Run(); err != nil { + log.Fatal(err) } + } From 2f342c5947d00e8864961c755de93ca013d67a51 Mon Sep 17 00:00:00 2001 From: daemon1024 Date: Wed, 24 Mar 2021 19:26:54 +0530 Subject: [PATCH 14/15] verify removals from parsed yaml files --- cmd/remove-members/main.go | 97 +++++++++++++++++++++++++++++--------- go.mod | 1 + go.sum | 4 ++ 3 files changed, 79 insertions(+), 23 deletions(-) diff --git a/cmd/remove-members/main.go b/cmd/remove-members/main.go index 33593fe037..b28d92765e 100644 --- a/cmd/remove-members/main.go +++ b/cmd/remove-members/main.go @@ -8,10 +8,11 @@ import ( "os/exec" "path/filepath" "regexp" - "sort" "strings" flag "github.com/spf13/pflag" + "k8s.io/test-infra/prow/config/org" + "sigs.k8s.io/yaml" ) type options struct { @@ -22,7 +23,7 @@ type options struct { func parseOptions() options { var o options flag.StringVar(&o.configPath, "path", ".", "Path to config directory/subdirectory") - flag.BoolVar(&o.confirm, "confirm", true, "Modify the actual files or just simulate changes") + flag.BoolVar(&o.confirm, "confirm", false, "Modify the actual files or just simulate changes") flag.Parse() @@ -69,7 +70,6 @@ func readMemberList(path string) ([]string, error) { func removeMembers(memberList []string, o options) error { for _, member := range memberList { var orgs, teams []string - count := 0 fmt.Print(member) if err := filepath.Walk(o.configPath, func(path string, info os.FileInfo, err error) error { @@ -83,20 +83,15 @@ func removeMembers(memberList []string, o options) error { if filepath.Ext(path) != ".yaml" { return nil } - removed, removeCount, err := removeMemberFromFile(member, path, info, o.confirm) + removed, removals, err := removeMemberFromFile(member, path, info, o.confirm) if err != nil { return err } //Record the org/team name when a member is removed from it if removed { - count += removeCount - if info.Name() == "org.yaml" { - orgs = append(orgs, filepath.Base(filepath.Dir(path))) - } - if info.Name() == "teams.yaml" { - teams = append(teams, filepath.Base(filepath.Dir(path))) - } + orgs = append(orgs, removals["orgs"]...) + teams = append(teams, removals["teams"]...) } return nil @@ -104,13 +99,22 @@ func removeMembers(memberList []string, o options) error { return err } - sort.Strings(orgs) - sort.Strings(teams) + if len(orgs) > 0 { + fmt.Printf("\n Found %s in %s org(s)", member, strings.Join(orgs, ", ")) + } else { + fmt.Printf("\n Found %s in no org", member) + } + + if len(teams) > 0 { + fmt.Printf("\n Found %s in %s team(s)", member, strings.Join(teams, ", ")) + } else { + fmt.Printf("\n Found %s in no team", member) + } - fmt.Printf("\n Orgs: %v\n Teams: %v\n Number of occurences: %d\n", orgs, teams, count) + fmt.Printf("\n Total number of occurences: %d\n", len(orgs)+len(teams)) //Proceed to committing changes if member is actually removed from somewhere - if count > 0 { + if len(orgs)+len(teams) > 0 { commitRemovedMembers(member, orgs, teams, o.confirm) } } @@ -118,23 +122,37 @@ func removeMembers(memberList []string, o options) error { return nil } -func removeMemberFromFile(member string, path string, info os.FileInfo, confirm bool) (bool, int, error) { +func removeMemberFromFile(member string, path string, info os.FileInfo, confirm bool) (bool, map[string][]string, error) { content, err := ioutil.ReadFile(path) if err != nil { - return false, 0, err + return false, nil, err } - re := regexp.MustCompile(`(\s+)?- ` + member + `(.*)?`) + var cfg org.Config + if err := yaml.Unmarshal(content, &cfg); err != nil { + return false, nil, err + } + + removals := map[string][]string{ + "orgs": fetchOrgRemovals(cfg, []string{}, member), + "teams": fetchTeamRemovals(cfg.Teams, []string{}, member), + } - matches := re.FindAllIndex(content, -1) + if len(removals["orgs"])+len(removals["teams"]) > 0 { + re := regexp.MustCompile(`(\s+)?- ` + member + `(.*)?`) - if len(matches) >= 1 { + matches := re.FindAllIndex(content, -1) + + //Making Sure count from parsed config and regex matches are same + if len(matches) != len(removals["orgs"])+len(removals["teams"]) { + log.Printf("\n\n Mismatch in regex count and removal count at %s\n", path) + } if confirm { updatedContent := re.ReplaceAll(content, []byte("")) if err = ioutil.WriteFile(path, updatedContent, info.Mode()); err != nil { - return false, len(matches), err + return false, removals, err } cmd := exec.Command("git", "add", path) @@ -143,10 +161,10 @@ func removeMemberFromFile(member string, path string, info os.FileInfo, confirm } } - return true, len(matches), nil + return true, removals, nil } - return false, len(matches), nil + return false, removals, nil } @@ -180,3 +198,36 @@ func commitRemovedMembers(member string, orgs []string, teams []string, confirm } } + +func fetchOrgRemovals(cfg org.Config, removals []string, member string) []string { + for _, i := range cfg.Members { + if i == member { + removals = append(removals, *cfg.Name) + } + } + for _, i := range cfg.Admins { + if i == member { + removals = append(removals, *cfg.Name) + } + } + return removals +} + +func fetchTeamRemovals(teams map[string]org.Team, removals []string, member string) []string { + for teamName, v := range teams { + for _, i := range v.Members { + if i == member { + removals = append(removals, teamName) + } + } + for _, i := range v.Maintainers { + if i == member { + removals = append(removals, teamName) + } + } + if len(v.Children) > 0 { + removals = fetchTeamRemovals(v.Children, removals, member) + } + } + return removals +} diff --git a/go.mod b/go.mod index 78d8229459..f5172b0525 100644 --- a/go.mod +++ b/go.mod @@ -8,6 +8,7 @@ require ( github.com/spf13/pflag v1.0.5 k8s.io/apimachinery v0.0.0-20190817020851-f2f3a405f61d k8s.io/test-infra v0.0.0-20191222193732-de81526abe72 + sigs.k8s.io/yaml v1.2.0 // indirect ) // Pin all k8s.io staging repositories to kubernetes-1.15.3. diff --git a/go.sum b/go.sum index d8c5bd6c67..cd3d32a62c 100644 --- a/go.sum +++ b/go.sum @@ -495,6 +495,8 @@ gopkg.in/yaml.v2 v2.2.1/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI= gopkg.in/yaml.v2 v2.2.2/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI= gopkg.in/yaml.v2 v2.2.7 h1:VUgggvou5XRW9mHwD/yXxIYSMtY0zoKQf/v226p2nyo= gopkg.in/yaml.v2 v2.2.7/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI= +gopkg.in/yaml.v2 v2.2.8 h1:obN1ZagJSUGI0Ek/LBmuj4SNLPfIny3KsKFopxRdj10= +gopkg.in/yaml.v2 v2.2.8/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI= gopkg.in/yaml.v3 v3.0.0-20190709130402-674ba3eaed22/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= gotest.tools v2.2.0+incompatible/go.mod h1:DsYFclhRJ6vuDpmuTbkuFWG+y2sxOXAzmJt81HFBacw= honnef.co/go/tools v0.0.0-20180728063816-88497007e858/go.mod h1:rf3lG4BRIbNafJWhAfAdb/ePZxsR/4RtNHQocxwk9r4= @@ -538,4 +540,6 @@ sigs.k8s.io/controller-runtime v0.3.0/go.mod h1:Cw6PkEg0Sa7dAYovGT4R0tRkGhHXpYij sigs.k8s.io/structured-merge-diff v0.0.0-20190302045857-e85c7b244fd2/go.mod h1:wWxsB5ozmmv/SG7nM11ayaAW51xMvak/t1r0CSlcokI= sigs.k8s.io/testing_frameworks v0.1.1/go.mod h1:VVBKrHmJ6Ekkfz284YKhQePcdycOzNH9qL6ht1zEr/U= sigs.k8s.io/yaml v1.1.0/go.mod h1:UJmg0vDUVViEyp3mgSv9WPwZCDxu4rQW1olrI1uml+o= +sigs.k8s.io/yaml v1.2.0 h1:kr/MCeFWJWTwyaHoR9c8EjH9OumOmoF9YGiZd7lFm/Q= +sigs.k8s.io/yaml v1.2.0/go.mod h1:yfXDCHCao9+ENCvLSE62v9VSji2MKu5jeNfTrofGhJc= vbom.ml/util v0.0.0-20180919145318-efcd4e0f9787/go.mod h1:so/NYdZXCz+E3ZpW0uAoCj6uzU2+8OWDFv/HxUSs7kI= From 686abd3eeea85f125331a702b676ede0a79596a3 Mon Sep 17 00:00:00 2001 From: daemon1024 Date: Wed, 24 Mar 2021 20:43:28 +0530 Subject: [PATCH 15/15] refactor fetching removals from list --- cmd/remove-members/main.go | 37 ++++++++++++++++--------------------- 1 file changed, 16 insertions(+), 21 deletions(-) diff --git a/cmd/remove-members/main.go b/cmd/remove-members/main.go index b28d92765e..2c112fbe9a 100644 --- a/cmd/remove-members/main.go +++ b/cmd/remove-members/main.go @@ -135,7 +135,7 @@ func removeMemberFromFile(member string, path string, info os.FileInfo, confirm } removals := map[string][]string{ - "orgs": fetchOrgRemovals(cfg, []string{}, member), + "orgs": fetchOrgRemovals(cfg, []string{}, member, path), "teams": fetchTeamRemovals(cfg.Teams, []string{}, member), } @@ -199,35 +199,30 @@ func commitRemovedMembers(member string, orgs []string, teams []string, confirm } -func fetchOrgRemovals(cfg org.Config, removals []string, member string) []string { - for _, i := range cfg.Members { - if i == member { - removals = append(removals, *cfg.Name) - } - } - for _, i := range cfg.Admins { - if i == member { - removals = append(removals, *cfg.Name) - } +func fetchOrgRemovals(cfg org.Config, removals []string, member string, path string) []string { + if cfg.Name != nil { + removals = fetchRemovals(cfg.Members, removals, member, *cfg.Name) + removals = fetchRemovals(cfg.Admins, removals, member, *cfg.Name) } return removals } func fetchTeamRemovals(teams map[string]org.Team, removals []string, member string) []string { for teamName, v := range teams { - for _, i := range v.Members { - if i == member { - removals = append(removals, teamName) - } - } - for _, i := range v.Maintainers { - if i == member { - removals = append(removals, teamName) - } - } + removals = fetchRemovals(v.Members, removals, member, teamName) + removals = fetchRemovals(v.Maintainers, removals, member, teamName) if len(v.Children) > 0 { removals = fetchTeamRemovals(v.Children, removals, member) } } return removals } + +func fetchRemovals(list []string, removals []string, member string, name string) []string { + for _, i := range list { + if i == member { + removals = append(removals, name) + } + } + return removals +}