Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
5 changes: 2 additions & 3 deletions .fasterci/config.yaml
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
workflows:

- &build_workflow
name: Faster CI / build (default bazel ver)
on:
Expand Down Expand Up @@ -43,6 +42,6 @@ workflows:
- --test_size_filters=-large,-enormous

- <<: *build_workflow
name: Faster CI / build (6.5.0)
name: Faster CI / build (7.4.1)
env:
USE_BAZEL_VERSION: "6.5.0"
USE_BAZEL_VERSION: "7.4.1"
1 change: 1 addition & 0 deletions e2e/MODULE.bazel
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
bazel_dep(name = "rules_go", version = "0.46.0", repo_name = "io_bazel_rules_go")
bazel_dep(name = "bazel_skylib", version = "1.5.0")
bazel_dep(name = "rules_oci", version = "1.7.2")

oci = use_extension("@rules_oci//oci:extensions.bzl", "oci")
oci.pull(
name = "go_image_static",
Expand Down
2 changes: 2 additions & 0 deletions gitops/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ load("//gitops/private:kustomize_toolchain.bzl", "current_kustomize_toolchain")

package(default_visibility = ["//visibility:public"])

exports_files(["create_gitops_prs.tpl.sh"])

toolchain_type(
name = "kustomize_toolchain_type",
)
Expand Down
7 changes: 7 additions & 0 deletions gitops/create_gitops_prs.tpl.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
#!/usr/bin/env bash
set -x
set -e

GIT_COMMIT=$(git rev-parse HEAD)

%{prer} --git_commit=$GIT_COMMIT %{params} "${@}"
5 changes: 3 additions & 2 deletions gitops/exec/exec.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,10 +31,11 @@ func Ex(dir, name string, arg ...string) (output string, err error) {

// Mustex executes the command name arg... in directory dir
// it will exit with fatal error if execution was not successful
func Mustex(dir, name string, arg ...string) {
_, err := Ex(dir, name, arg...)
func Mustex(dir, name string, arg ...string) string {
ret, err := Ex(dir, name, arg...)
if err != nil {
log.Fatalf("ERROR: %s", err)
Comment on lines 36 to 37
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The use of log.Fatalf inside the Mustex function to handle errors is problematic for a couple of reasons. Firstly, it will cause the entire program to exit if the command execution fails, which might not be the desired behavior in all contexts where Mustex is used. This approach reduces the flexibility of error handling, making it impossible for calling code to recover from the error or take alternative actions. Secondly, abruptly terminating the program can make it harder to perform cleanup operations or gracefully shutdown other parts of the application.

Recommended Solution: Instead of terminating the program, consider returning the error to the caller. This allows the caller to decide how to handle the error, whether it be logging the error, retrying the operation, or gracefully shutting down. If the intention is to have a version of the function that does not return an error to the caller, you could provide two versions of the function: one that returns an error and one that does not, clearly documenting the behavior of each.

}
return ret

}
58 changes: 50 additions & 8 deletions gitops/git/git.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,19 +13,15 @@ package git

import (
"fmt"
"io/ioutil"
"log"
"os"
oe "os/exec"
"path/filepath"
"strings"

"github.com/fasterci/rules_gitops/gitops/exec"
)

var (
git = "git"
)

// Clone clones a repository. Pass the full repository name, such as
// "https://aleksey.pesternikov@bitbucket.tubemogul.info/scm/tm/repo.git" as the repo.
// Cloned directory will be clean of local changes with primaryBranch branch checked out.
Expand All @@ -34,7 +30,7 @@ var (
// mirrorDir: optional (if not empty) local mirror of the repository
func Clone(repo, dir, mirrorDir, primaryBranch, gitopsPath string) (*Repo, error) {
if err := os.RemoveAll(dir); err != nil {
return nil, fmt.Errorf("Unable to clone repo: %w", err)
return nil, fmt.Errorf("unable to clone repo: %w", err)
Comment on lines 32 to +33
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The use of os.RemoveAll(dir) to clean the directory before cloning the repository poses a significant risk of data loss if the dir variable contains an incorrect path, especially if it points to a critical directory. A safer approach would be to verify the contents of the directory or ensure it is within a controlled environment before deletion. Consider implementing checks or confirmations before performing such destructive operations.

}
if mirrorDir != "" {
exec.Mustex("", "git", "clone", "-n", "--reference", mirrorDir, repo, dir)
Expand All @@ -43,8 +39,8 @@ func Clone(repo, dir, mirrorDir, primaryBranch, gitopsPath string) (*Repo, error
}
exec.Mustex(dir, "git", "config", "--local", "core.sparsecheckout", "true")
genPath := fmt.Sprintf("%s/\n", gitopsPath)
if err := ioutil.WriteFile(filepath.Join(dir, ".git/info/sparse-checkout"), []byte(genPath), 0644); err != nil {
return nil, fmt.Errorf("Unable to create .git/info/sparse-checkout: %w", err)
if err := os.WriteFile(filepath.Join(dir, ".git/info/sparse-checkout"), []byte(genPath), 0644); err != nil {
return nil, fmt.Errorf("unable to create .git/info/sparse-checkout: %w", err)
}
exec.Mustex(dir, "git", "checkout", primaryBranch)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The function exec.Mustex is used without error handling, which could lead to the program continuing execution even if a critical error occurs during the git operations (e.g., clone, config, checkout). This could result in a partially cloned or misconfigured repository without the user being aware of the failure. It is recommended to handle errors returned by these operations to ensure the integrity of the cloning process and provide clear feedback to the user in case of failure.


Expand All @@ -53,6 +49,52 @@ func Clone(repo, dir, mirrorDir, primaryBranch, gitopsPath string) (*Repo, error
}, nil
}

func CloneOrCheckout(repo, dir, mirrorDir, primaryBranch, gitopsPath, branchPrefix string) (r *Repo, err error) {
newRepo := false
if _, err = os.Stat(dir + "/.git"); os.IsNotExist(err) {
newRepo = true
if err = os.MkdirAll(filepath.Dir(dir), os.ModePerm); err != nil && !os.IsExist(err) {
return nil, err
}
Comment on lines +54 to +58
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The error handling for os.MkdirAll checks for !os.IsExist(err) which might not be sufficient to catch all relevant errors. Specifically, this condition only checks if the error is not about the directory already existing, but it does not ensure that the directory was successfully created or that it is writable. This could lead to a scenario where the directory creation fails for a reason other than it already existing, but the error is not properly handled, potentially leading to more cryptic errors downstream when trying to use the directory.

A more robust approach would be to handle any error returned by os.MkdirAll directly, without trying to filter it based on its type. If the directory's existence is crucial for the operation to proceed, additional checks should be performed to ensure not only its existence but also its accessibility and suitability for the intended operations.

Comment on lines +54 to +58
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The error handling for os.MkdirAll checks for !os.IsExist(err) which might not be sufficient to catch all relevant errors. Specifically, this condition only checks if the error is not about the directory already existing, but it does not ensure that the directory was successfully created or that it is writable. This could lead to a scenario where the directory creation fails for a reason other than it already existing, but the error is not properly handled, potentially leading to more cryptic errors downstream when trying to use the directory.

A more robust approach would be to handle any error returned by os.MkdirAll directly, without trying to filter it based on its type. If the directory's existence is crucial for the operation to proceed, additional checks should be performed to ensure not only its existence but also its accessibility and suitability for the intended operations.

if mirrorDir != "" {
exec.Mustex("", "git", "clone", "-n", "--reference", mirrorDir, repo, dir)
} else {
exec.Mustex("", "git", "clone", "-n", repo, dir)
}
} else {
//existing repo
exec.Mustex(dir, "git", "remote", "set-url", "origin", repo)
exec.Mustex(dir, "git", "reset", "--hard")
}
exec.Mustex(dir, "git", "checkout", "-f", primaryBranch)
if !newRepo {
exec.Mustex(dir, "git", "fetch", "origin", "--prune")
DeleteLocalBranches(dir, branchPrefix)
}

return &Repo{
Dir: dir,
}, nil
}
Comment on lines +52 to +78
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The function exec.Mustex is used without error handling, which could lead to the program continuing execution even if a critical error occurs during the git operations (e.g., clone, config, checkout). This could result in a partially cloned or misconfigured repository without the user being aware of the failure. It is recommended to handle errors returned by these operations to ensure the integrity of the cloning process and provide clear feedback to the user in case of failure.


// DeleteLocalBranches removes local branches by prefix.
func DeleteLocalBranches(dir, branchprefix string) {
branches := exec.Mustex(dir, "git", "for-each-ref", "--format", "%(refname)", "refs/heads/"+branchprefix)
// returned format:
// refs/heads/deploy/dev
// refs/heads/deploy/prod
// refs/heads/master
v := strings.Split(branches, "\n")
for _, line := range v {
ref := strings.TrimSpace(line)
if strings.HasPrefix(ref, "refs/heads/"+branchprefix) {
ref = strings.TrimPrefix(ref, "refs/heads/")
exec.Mustex(dir, "git", "branch", "-D", ref)
}

}
}

// Repo is a clone of a git repository. Create with Clone, and don't
// forget to clean it up after.
type Repo struct {
Expand Down
132 changes: 132 additions & 0 deletions gitops/gitops.bzl
Original file line number Diff line number Diff line change
@@ -0,0 +1,132 @@
load("@rules_gitops//gitops:provider.bzl", "GitopsArtifactsInfo")

def __create_gitops_prs_impl(ctx):
src_by_train = {}
for src in ctx.attr.srcs:
gai = src[GitopsArtifactsInfo]
if not gai.deployment_branch in src_by_train:
src_by_train[gai.deployment_branch] = []
src_by_train[gai.deployment_branch].append(src.files_to_run.executable)

# print("src_by_train:", src_by_train)
trans_img_pushes = depset(transitive = [obj[GitopsArtifactsInfo].image_pushes for obj in ctx.attr.srcs if obj.files_to_run.executable]).to_list()
params = ""
for deployment_branch in src_by_train.keys():
executables = src_by_train[deployment_branch]
for exe in executables:
params += "--resolved_binary {}:{} ".format(deployment_branch, exe.short_path)
for exe in trans_img_pushes:
params += "--resolved_push {} ".format(exe.files_to_run.executable.short_path)
if ctx.attr.release_branch:
params += "--release_branch {} ".format(ctx.attr.release_branch)
if ctx.attr.git_repo:
params += "--git_repo {} ".format(ctx.attr.git_repo)
if ctx.attr.gitops_path:
params += "--gitops_path {} ".format(ctx.attr.gitops_path)
if ctx.attr.gitopsdir:
params += "--gitopsdir {} ".format(ctx.attr.gitopsdir)
params += "--push_parallelism {} ".format(ctx.attr.push_parallelism)
if ctx.attr.gitops_pr_into:
params += "--gitops_pr_into {} ".format(ctx.attr.gitops_pr_into)
if ctx.attr.deploy_branch_prefix:
params += "--deploy_branch_prefix {} ".format(ctx.attr.deploy_branch_prefix)
if ctx.attr.deployment_branch_suffix:
params += "--deployment_branch_suffix {} ".format(ctx.attr.deployment_branch_suffix)
if ctx.attr.git_server:
params += "--git_server {} ".format(ctx.attr.git_server)
if ctx.attr.dry_run:
params += "--dry_run "
if ctx.attr.github_repo_owner:
params += "--github_repo_owner {} ".format(ctx.attr.github_repo_owner)
if ctx.attr.github_repo:
params += "--github_repo {} ".format(ctx.attr.github_repo)

ctx.actions.expand_template(
template = ctx.file._tpl,
substitutions = {
"%{params}": params,
"%{prer}": ctx.executable._prer.short_path,
},
output = ctx.outputs.executable,
)
runfiles = ctx.runfiles(files = ctx.files.srcs)
transitive_runfiles = []
for target in ctx.attr.srcs:
transitive_runfiles.append(target[DefaultInfo].default_runfiles)
transitive_runfiles.append(ctx.attr._prer[DefaultInfo].default_runfiles)
for obj in trans_img_pushes:
transitive_runfiles.append(obj[DefaultInfo].default_runfiles)
runfiles = runfiles.merge_all(transitive_runfiles)
return [
DefaultInfo(
executable = ctx.outputs.executable,
runfiles = runfiles,
),
]

create_gitops_prs = rule(
doc = """Rule to all required gitops prs for a given k8s_deploy targets
Typical usage is to combine this rule with k8s_deploy to create a single executable that
push images to registry and creates gitops prs
""",
implementation = __create_gitops_prs_impl,
attrs = {
"srcs": attr.label_list(
mandatory = True,
allow_files = False,
providers = [GitopsArtifactsInfo],
doc = "List of gitops targets generated by k8s_deploy. Use update_gitops_targets.sh to update",
),
"git_repo": attr.string(
doc = "gitops repo to create PRs in.",
),
"gitops_path": attr.string(
doc = "location to store files in repo.",
),
"gitopsdir": attr.string(
doc = "do not use temporary directory for gitops, use this directory instead.",
),
"push_parallelism": attr.int(
doc = "number of parallel pushes to registry",
default = 4,
),
"gitops_pr_into": attr.string(
doc = "use this branch as the source branch and target for deployment PR",
default = "main",
),
"release_branch": attr.string(
doc = "release branch to create PRs in.",
),
"deploy_branch_prefix": attr.string(
doc = "prefix for deployment branches",
),
"deployment_branch_suffix": attr.string(
doc = "suffix for deployment branches",
),
"git_server": attr.string(
doc = "git server to create PRs in.",
default = "github",
),
"dry_run": attr.bool(
doc = "dry run mode",
default = False,
),
"github_repo_owner": attr.string(
doc = "github repo owner to create PRs in.",
),
"github_repo": attr.string(
doc = "github repo to create PRs in.",
),
"_tpl": attr.label(
default = Label("@rules_gitops//gitops:create_gitops_prs.tpl.sh"),
allow_single_file = True,
),
"_prer": attr.label(
default = Label("@rules_gitops//gitops/prer:create_gitops_prs"),
allow_single_file = True,
cfg = "exec",
executable = True,
),
},
executable = True,
)
1 change: 1 addition & 0 deletions gitops/prer/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ go_library(
"//gitops/git/github:go_default_library",
"//gitops/git/gitlab:go_default_library",
"//vendor/github.com/golang/protobuf/proto:go_default_library",
"//vendor/golang.org/x/sync/errgroup:go_default_library",
],
)

Expand Down
Loading