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

helm repo source #263

Merged
merged 3 commits into from
Jun 7, 2021
Merged
Show file tree
Hide file tree
Changes from 2 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
3 changes: 2 additions & 1 deletion cmd/wego/add/cmd.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,10 +33,11 @@ var Cmd = &cobra.Command{
func init() {
Cmd.Flags().StringVar(&params.Owner, "owner", "", "Owner of remote git repository")
Cmd.Flags().StringVar(&params.Name, "name", "", "Name of remote git repository")
Cmd.Flags().StringVar(&params.Url, "url", "", "URL of remote git repository")
Cmd.Flags().StringVar(&params.Url, "url", "", "URL of remote repository")
Cmd.Flags().StringVar(&params.Path, "path", "./", "Path of files within git repository")
Cmd.Flags().StringVar(&params.Branch, "branch", "main", "Branch to watch within git repository")
Cmd.Flags().StringVar(&params.DeploymentType, "deployment-type", "kustomize", "deployment type [kustomize, helm]")
Cmd.Flags().StringVar(&params.Chart, "chart", "", "Specify chart for --helm-repository source")
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like this description is no longer accurate (at least the "--" part)

Cmd.Flags().StringVar(&params.PrivateKey, "private-key", filepath.Join(os.Getenv("HOME"), ".ssh", "id_rsa"), "Private key that provides access to git repository")
Cmd.Flags().BoolVar(&params.DryRun, "dry-run", false, "If set, 'wego add' will not make any changes to the system; it will just display the actions that would have been taken")
Cmd.Flags().BoolVar(&params.IsPrivate, "private", true, "Set access control on the repo")
Expand Down
82 changes: 65 additions & 17 deletions pkg/cmdimpl/add.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ type AddParamSet struct {
PrivateKey string
PrivateKeyPass string
DeploymentType string
Chart string
Namespace string
DryRun bool
IsPrivate bool
Expand Down Expand Up @@ -194,7 +195,7 @@ func generateWegoKustomizeManifest() ([]byte, error) {
return kustomizeManifest, nil
}

func generateSourceManifest() ([]byte, error) {
func generateSourceManifestGit() ([]byte, error) {
secretName := params.Name

cmd := fmt.Sprintf(`create secret git "%s" \
Expand Down Expand Up @@ -235,6 +236,23 @@ func generateSourceManifest() ([]byte, error) {
return sourceManifest, nil
}

func generateSourceManifestHelm() ([]byte, error) {
cmd := fmt.Sprintf(`create source helm %s \
--url="%s" \
--interval=30s \
--export \
--namespace=%s `,
params.Name,
params.Url,
params.Namespace)

sourceManifest, err := fluxops.CallFlux(cmd)
if err != nil {
return nil, wrapError(err, "could not create git source")
}
return sourceManifest, nil
}

func generateKustomizeManifest() ([]byte, error) {
cmd := fmt.Sprintf(`create kustomization "%s" \
--path="%s" \
Expand All @@ -256,7 +274,7 @@ func generateKustomizeManifest() ([]byte, error) {
return kustomizeManifest, nil
}

func generateHelmManifest() ([]byte, error) {
func generateHelmManifestGit() ([]byte, error) {
cmd := fmt.Sprintf(`create helmrelease %s \
--source="GitRepository/%s" \
--chart="%s" \
Expand All @@ -271,6 +289,22 @@ func generateHelmManifest() ([]byte, error) {
return fluxops.CallFlux(cmd)
}

func generateHelmManifestHelm() ([]byte, error) {
cmd := fmt.Sprintf(`create helmrelease %s \
--source="HelmRepository/%s" \
--chart="%s" \
--interval=5m \
--export \
--namespace=%s`,
params.Name,
params.Name,
params.Chart,
params.Namespace,
)

return fluxops.CallFlux(cmd)
}

func getOwner() (string, error) {
owner, err := fluxops.GetOwnerFromEnv()
if err != nil || owner == "" {
Expand Down Expand Up @@ -433,22 +467,36 @@ func Add(args []string, allParams AddParamSet, deps *AddDependencies) error {
}

// Create flux custom resources for new repo being added
source, err := generateSourceManifest()
if err != nil {
return wrapError(err, "could not generate source manifest")
}

var source []byte
var appManifests []byte
switch params.DeploymentType {
case string(DeployTypeHelm):
appManifests, err = generateHelmManifest()
case string(DeployTypeKustomize):
appManifests, err = generateKustomizeManifest()
default:
return fmt.Errorf("deployment type not supported: %s", params.DeploymentType)
}
if err != nil {
return wrapError(err, "error generating manifest")

// If chart is set ignore deployment type. Going to revisit later
if params.Chart != "" {
source, err = generateSourceManifestHelm()
if err != nil {
return wrapError(err, "could not generate source manifest")
Copy link
Contributor

Choose a reason for hiding this comment

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

would it be possible to add something here to differentiate this error from the next one (both are the same) ?. Just for easy debugging in the future.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I mean the error that happens depends on which path you choose. If you are adding a helm repo then you know the error was in helm.

}
appManifests, err = generateHelmManifestHelm()
if err != nil {
return wrapError(err, "error generating manifest")
}
} else {
source, err = generateSourceManifestGit()
Copy link
Contributor

Choose a reason for hiding this comment

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

huge favor, could you please help me make this function to use the new Chart field instead of Path. When I created it, I used Path but it seems more accurate to use Chart now.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I talked with @jrryjcksn at least on whether or not i should use path. We agreed for helm-repository chart made sense but I didnt really ask about using chart for github.

Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like Flux asks the user for "chart" in either case so we should probably follow that lead.

if err != nil {
return wrapError(err, "could not generate source manifest")
}

switch params.DeploymentType {
case string(DeployTypeHelm):
appManifests, err = generateHelmManifestGit()
case string(DeployTypeKustomize):
appManifests, err = generateKustomizeManifest()
default:
return fmt.Errorf("deployment type not supported: %s", params.DeploymentType)
}
if err != nil {
return wrapError(err, "error generating manifest")
}
}

appSubdir := filepath.Join("apps", params.Name)
Expand Down
63 changes: 59 additions & 4 deletions pkg/cmdimpl/add_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -173,8 +173,8 @@ func handleGitLsRemote(arglist ...interface{}) ([]byte, []byte, error) {

var fakeGitClient = gitfakes.FakeGit{}

var _ = Describe("Test helm manifest", func() {
It("Verify helm manifest files generation ", func() {
var _ = Describe("Test helm manifest from git repo", func() {
It("Verify helm manifest files generation from git ", func() {

expected := `create helmrelease simple-name \
--source="GitRepository/simple-name" \
Expand All @@ -192,15 +192,70 @@ var _ = Describe("Test helm manifest", func() {

fluxops.SetFluxHandler(fakeHandler)

params.Name = "simple-name"
params.Name = "simple-name"
params.Path = "./my-chart"
params.Namespace = "wego-system"

Expect(generateHelmManifest()).Should(Equal([]byte("foo")))
Expect(generateHelmManifestGit()).Should(Equal([]byte("foo")))
})
})

var _ = Describe("Test helm manifest from helm repo", func() {
It("Verify helm manifest generation from helm ", func() {

expected := `create helmrelease simple-name \
--source="HelmRepository/simple-name" \
--chart="testchart" \
--interval=5m \
--export \
--namespace=wego-system`

fakeHandler := &fluxopsfakes.FakeFluxHandler{
HandleStub: func(args string) ([]byte, error) {
Expect(args).Should(Equal(expected))
return []byte("foo"), nil
},
}

fluxops.SetFluxHandler(fakeHandler)

params.Name = "simple-name"
params.Namespace = "wego-system"
params.Chart = "testchart"

Expect(generateHelmManifestHelm()).Should(Equal([]byte("foo")))
})

})

var _ = Describe("Test helm source from helm repo", func() {
It("Verify helm source generation from helm ", func() {

expected := `create source helm test \
--url="https://github.io/testrepo" \
--interval=30s \
--export \
--namespace=wego-system `

fakeHandler := &fluxopsfakes.FakeFluxHandler{
HandleStub: func(args string) ([]byte, error) {
Expect(args).Should(Equal(expected))
return []byte("foo"), nil
},
}

fluxops.SetFluxHandler(fakeHandler)
Copy link
Contributor

Choose a reason for hiding this comment

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

I made a change in the other helm test in my stuff to not set the fake handler directly (using WithOverrides instead) because it leaves the flux handler broken for other tests.


params.Name = "test"
params.Url = "https://github.io/testrepo"
params.Namespace = "wego-system"
params.Chart = "testChart"

Expect(generateSourceManifestHelm()).Should(Equal([]byte("foo")))
})

})

var _ = Describe("Dry Run Add Test", func() {
It("Verify that the dry-run flag leaves clusters and repos unchanged", func() {
By("Executing a dry-run add and failing/exiting if any of the flux actions were invoked", func() {
Expand Down