Skip to content

*: add back service account #629

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

Merged
merged 4 commits into from
Oct 17, 2018
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
4 changes: 3 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -76,10 +76,11 @@ $ docker push quay.io/example/app-operator
# Update the operator manifest to use the built image name
$ sed -i 's|REPLACE_IMAGE|quay.io/example/app-operator|g' deploy/operator.yaml

# Setup Service Account
$ kubectl create -f deploy/service_account.yaml
# Setup RBAC
$ kubectl create -f deploy/role.yaml
$ kubectl create -f deploy/role_binding.yaml
# TODO: kubectl create -f deploy/service_account.yaml
# Setup the CRD
$ kubectl create -f deploy/crds/app_v1alpha1_appservice_crd.yaml
# Deploy the app-operator
Expand All @@ -99,6 +100,7 @@ $ kubectl delete -f deploy/app_v1alpha1_appservice_cr.yaml
$ kubectl delete -f deploy/operator.yaml
$ kubectl delete -f deploy/role.yaml
$ kubectl delete -f deploy/role_binding.yaml
$ kubectl delete -f deploy/service_account.yaml
$ kubectl delete -f deploy/crds/app_v1alpha1_appservice_crd.yaml
```

Expand Down
2 changes: 2 additions & 0 deletions commands/operator-sdk/cmd/new.go
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,7 @@ func doScaffold() {
err := s.Execute(cfg,
&scaffold.Cmd{},
&scaffold.Dockerfile{},
&scaffold.ServiceAccount{},
&scaffold.Role{},
&scaffold.RoleBinding{},
&scaffold.Operator{},
Expand Down Expand Up @@ -180,6 +181,7 @@ func doAnsibleScaffold() {
GeneratePlaybook: generatePlaybook,
},
galaxyInit,
&scaffold.ServiceAccount{},
&scaffold.Role{},
&scaffold.RoleBinding{},
&ansible.Operator{},
Expand Down
22 changes: 8 additions & 14 deletions commands/operator-sdk/cmd/test/local.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,21 +63,18 @@ func testLocalFunc(cmd *cobra.Command, args []string) {
if len(args) != 1 {
log.Fatalf("operator-sdk test local requires exactly 1 argument")
}
// if no namespaced manifest path is given, combine deploy/sa.yaml, deploy/rbac.yaml and deploy/operator.yaml
// if no namespaced manifest path is given, combine deploy/service_account.yaml, deploy/role.yaml, deploy/role_binding.yaml and deploy/operator.yaml
if tlConfig.namespacedManPath == "" {
err := os.MkdirAll("deploy/test", os.FileMode(cmdutil.DefaultDirFileMode))
if err != nil {
log.Fatalf("could not create deploy/test: %v", err)
}
tlConfig.namespacedManPath = "deploy/test/namespace-manifests.yaml"

// TODO: re-enable sa creation once that's added to the refactor branch
/*
sa, err := ioutil.ReadFile("deploy/sa.yaml")
if err != nil {
log.Fatalf("could not find sa manifest: %v", err)
}
*/
sa, err := ioutil.ReadFile("deploy/service_account.yaml")
if err != nil {
log.Fatalf("could not find the manifest deploy/service_account.yaml: %v", err)
}
role, err := ioutil.ReadFile("deploy/role.yaml")
if err != nil {
log.Fatalf("could not find role manifest: %v", err)
Expand All @@ -90,12 +87,9 @@ func testLocalFunc(cmd *cobra.Command, args []string) {
if err != nil {
log.Fatalf("could not find operator manifest: %v", err)
}
/*
combined := append(sa, []byte("\n---\n")...)
combined = append(combined, rbac...)
combined = append(combined, []byte("\n---\n")...)
*/
combined := append(role, []byte("\n---\n")...)
combined := append(sa, []byte("\n---\n")...)
combined = append(combined, role...)
combined = append(combined, []byte("\n---\n")...)
combined = append(combined, roleBinding...)
combined = append(combined, []byte("\n---\n")...)
combined = append(combined, operator...)
Expand Down
16 changes: 11 additions & 5 deletions doc/ansible/user-guide.md
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,7 @@ resource definition the operator will be watching.
Deploy the CRD:

```sh
$ kubectl create -f deploy/crd.yaml
$ kubectl create -f deploy/crds/cache_v1alpha1_memcached_crd.yaml
```

Once this is done, there are two ways to run the operator:
Expand Down Expand Up @@ -207,7 +207,9 @@ $ sed -i 's|REPLACE_IMAGE|quay.io/example/memcached-operator:v0.0.1|g' deploy/op
Deploy the memcached-operator:

```sh
$ kubectl create -f deploy/rbac.yaml
$ kubectl create -f deploy/service_account.yaml
Copy link
Member

Choose a reason for hiding this comment

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

+1 thank you for updating this file

$ kubectl create -f deploy/role.yaml
$ kubectl create -f deploy/role_binding.yaml
$ kubectl create -f deploy/operator.yaml
```

Expand Down Expand Up @@ -268,7 +270,7 @@ metadata:
spec:
size: 3

$ kubectl apply -f deploy/cr.yaml
$ kubectl apply -f deploy/crds/cache_v1alpha1_memcached_cr.yaml
```

Ensure that the memcached-operator creates the deployment for the CR:
Expand Down Expand Up @@ -305,7 +307,7 @@ metadata:
spec:
size: 4

$ kubectl apply -f deploy/cr.yaml
$ kubectl apply -f deploy/crds/cache_v1alpha1_memcached_cr.yaml
```

Confirm that the operator changes the deployment size:
Expand All @@ -321,8 +323,12 @@ example-memcached 4 4 4 4 5m
Clean up the resources:

```sh
$ kubectl delete -f deploy/cr.yaml
$ kubectl delete -f deploy/crds/cache_v1alpha1_memcached_cr.yaml
$ kubectl delete -f deploy/operator.yaml
$ kubectl delete -f deploy/role_binding.yaml
$ kubectl delete -f deploy/role.yaml
$ kubectl delete -f deploy/service_account.yaml
$ kubectl delete -f deploy/crds/cache_v1alpha1_memcached_cr.yaml
```

[layout_doc]:./project_layout.md
Expand Down
2 changes: 1 addition & 1 deletion doc/dev/testing/end-to-end.md
Original file line number Diff line number Diff line change
Expand Up @@ -47,4 +47,4 @@ testing process, the cleanup functions will not run. To manually clean up a test
1. Delete the created project in $GOPATH/src/github.com/example-inc/memcached-operator
2. Delete the namespaces that the tests run in, which also deletes any resources created
within the namespaces. The namespaces start with `memcached-memcached-group`.
3. Delete the CRD (`kubectl delete -f deploy/crd.yaml`).
3. Delete the CRD (`kubectl delete -f deploy/crds/cache_v1alpha1_memcached_crd.yaml`).
2 changes: 1 addition & 1 deletion doc/sdk-cli-reference.md
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,7 @@ Runs the tests locally
##### Flags
* `--kubeconfig` string - location of kubeconfig for kubernetes cluster (default "~/.kube/config")
* `--global-manifest` string - path to manifest for global resources (default "deploy/crd.yaml)
* `--namespaced-manifest` string - path to manifest for per-test, namespaced resources (default: combines deploy/sa.yaml, deploy/rbac.yaml, and deploy/operator.yaml)
* `--namespaced-manifest` string - path to manifest for per-test, namespaced resources (default: combines deploy/service_account.yaml, deploy/rbac.yaml, and deploy/operator.yaml)
* `--namespace` string - if non-empty, single namespace to run tests in (e.g. "operator-test") (default: "")
* `--go-test-flags` string - extra arguments to pass to `go test` (e.g. -f "-v -parallel=2")
* `-h, --help` - help for local
Expand Down
15 changes: 8 additions & 7 deletions doc/test-framework/writing-e2e-tests.md
Original file line number Diff line number Diff line change
Expand Up @@ -224,7 +224,7 @@ functions will automatically be run since they were deferred when the TestCtx wa

To make running the tests simpler, the `operator-sdk` CLI tool has a `test` subcommand that can configure
default test settings, such as locations of your global resource manifest file (by default
`deploy/crd.yaml`) and your namespaced resource manifest file (by default `deploy/sa.yaml` concatenated with
`deploy/crd.yaml`) and your namespaced resource manifest file (by default `deploy/service_account.yaml` concatenated with
`deploy/rbac.yaml` and `deploy/operator.yaml`), and allows the user to configure runtime options. There are 2 ways to use the
subcommand: local and cluster.
### Local
Expand All @@ -249,8 +249,8 @@ in [MainEntry][main-entry-link] are declared, the tests will run correctly. Runn
will result in undefined behavior. This is an example `go test` equivalent to the `operator-sdk test local` example above:

```shell
# Combine sa, rbac, operator manifest into namespaced manifest
$ cp deploy/sa.yaml deploy/namespace-init.yaml
# Combine service_account, rbac, operator manifest into namespaced manifest
$ cp deploy/service_account.yaml deploy/namespace-init.yaml
$ echo -e "\n---\n" >> deploy/namespace-init.yaml
$ cat deploy/rbac.yaml >> deploy/namespace-init.yaml
$ echo -e "\n---\n" >> deploy/namespace-init.yaml
Expand All @@ -277,10 +277,11 @@ Once the image is ready, the tests are ready to be run. To run the tests, make s
and a namespace with proper rbac configured:

```shell
$ kubectl create -f deploy/crd.yaml
$ kubectl create -f deploy/crds/cache_v1alpha1_memcached_crd.yaml
$ kubectl create namespace memcached-test
$ kubectl create -f deploy/sa.yaml -n memcached-test
$ kubectl create -f deploy/rbac.yaml -n memcached-test
$ kubectl create -f deploy/service_account.yaml -n memcached-test
$ kubectl create -f deploy/role.yaml -n memcached-test
$ kubectl create -f deploy/role_binding.yaml -n memcached-test
```

Once you have your environment properly configured, you can start the tests using the `operator-sdk test cluster` command:
Expand Down Expand Up @@ -328,7 +329,7 @@ $ kubectl delete namespace main-153428703
Since the CRD is not namespaced, it must be deleted separately. Clean up the CRD created by the tests using the CRD manifest `deploy/crd.yaml`:

```shell
$ kubectl delete -f deploy/crd.yaml
$ kubectl delete -f deploy/crds/cache_v1alpha1_memcached_crd.yaml
```

[memcached-sample]:https://github.com/operator-framework/operator-sdk-samples/tree/master/memcached-operator
Expand Down
5 changes: 4 additions & 1 deletion doc/user-guide.md
Original file line number Diff line number Diff line change
Expand Up @@ -185,9 +185,9 @@ The Deployment manifest is generated at `deploy/operator.yaml`. Be sure to updat
Setup RBAC and deploy the memcached-operator:

```sh
$ kubectl create -f deploy/service_account.yaml
$ kubectl create -f deploy/role.yaml
$ kubectl create -f deploy/role_binding.yaml
# TODO: $ kubectl create -f deploy/service_account.yaml
$ kubectl create -f deploy/operator.yaml
```

Expand Down Expand Up @@ -311,6 +311,9 @@ Clean up the resources:
```sh
$ kubectl delete -f deploy/crds/cache_v1alpha1_memcached_cr.yaml
$ kubectl delete -f deploy/operator.yaml
$ kubectl delete -f deploy/role_binding.yaml
$ kubectl delete -f deploy/role.yaml
$ kubectl delete -f deploy/service_account.yaml
```

## Advanced Topics
Expand Down
1 change: 1 addition & 0 deletions pkg/scaffold/ansible/operator.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ spec:
labels:
name: {{.ProjectName}}
spec:
serviceAccountName: {{.ProjectName}}
containers:
- name: {{.ProjectName}}
# Replace this with the built image name
Expand Down
1 change: 1 addition & 0 deletions pkg/scaffold/constants.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ const (
versionFile = "version.go"
docFile = "doc.go"
registerFile = "register.go"
serviceAccountYamlFile = "service_account.yaml"
roleYamlFile = "role.yaml"
roleBindingYamlFile = "role_binding.yaml"
operatorYamlFile = "operator.yaml"
Expand Down
1 change: 0 additions & 1 deletion pkg/scaffold/gopkgtoml.go
Original file line number Diff line number Diff line change
Expand Up @@ -84,5 +84,4 @@ required = [
[[prune.project]]
name = "k8s.io/code-generator"
non-go = false
unused-packages = false
`
1 change: 0 additions & 1 deletion pkg/scaffold/gopkgtoml_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -88,5 +88,4 @@ required = [
[[prune.project]]
name = "k8s.io/code-generator"
non-go = false
unused-packages = false
`
1 change: 1 addition & 0 deletions pkg/scaffold/operator.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ spec:
labels:
name: {{.ProjectName}}
spec:
serviceAccountName: {{.ProjectName}}
containers:
- name: {{.ProjectName}}
# Replace this with the built image name
Expand Down
1 change: 1 addition & 0 deletions pkg/scaffold/operator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ spec:
labels:
name: app-operator
spec:
serviceAccountName: app-operator
containers:
- name: app-operator
# Replace this with the built image name
Expand Down
4 changes: 2 additions & 2 deletions pkg/scaffold/rolebinding.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,10 +35,10 @@ func (s *RoleBinding) GetInput() (input.Input, error) {
const roleBindingTemplate = `kind: RoleBinding
apiVersion: rbac.authorization.k8s.io/v1
metadata:
name: default-account-{{.ProjectName}}
name: {{.ProjectName}}
subjects:
- kind: ServiceAccount
name: default
name: {{.ProjectName}}
roleRef:
kind: Role
name: {{.ProjectName}}
Expand Down
4 changes: 2 additions & 2 deletions pkg/scaffold/rolebinding_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,10 +37,10 @@ func TestRoleBinding(t *testing.T) {
const rolebindingExp = `kind: RoleBinding
apiVersion: rbac.authorization.k8s.io/v1
metadata:
name: default-account-app-operator
name: app-operator
subjects:
- kind: ServiceAccount
name: default
name: app-operator
roleRef:
kind: Role
name: app-operator
Expand Down
39 changes: 39 additions & 0 deletions pkg/scaffold/service_account.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
// Copyright 2018 The Operator-SDK Authors
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

package scaffold

import (
"path/filepath"

"github.com/operator-framework/operator-sdk/pkg/scaffold/input"
)

type ServiceAccount struct {
input.Input
}

func (s *ServiceAccount) GetInput() (input.Input, error) {
if s.Path == "" {
s.Path = filepath.Join(deployDir, serviceAccountYamlFile)
}
s.TemplateBody = serviceAccountTemplate
return s.Input, nil
}

const serviceAccountTemplate = `apiVersion: v1
kind: ServiceAccount
metadata:
name: {{.ProjectName}}
`
41 changes: 41 additions & 0 deletions pkg/scaffold/service_account_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
// Copyright 2018 The Operator-SDK Authors
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

package scaffold

import (
"testing"

"github.com/sergi/go-diff/diffmatchpatch"
)

func TestServiceAccount(t *testing.T) {
s, buf := setupScaffoldAndWriter()
err := s.Execute(appConfig, &ServiceAccount{})
if err != nil {
t.Fatalf("failed to execute the scaffold: (%v)", err)
}

if serviceAccountExp != buf.String() {
dmp := diffmatchpatch.New()
diffs := diffmatchpatch.New().DiffMain(serviceAccountExp, buf.String(), false)
t.Fatalf("expected vs actual differs. Red text is missing and green text is extra.\n%v", dmp.DiffPrettyText(diffs))
Copy link
Member

Choose a reason for hiding this comment

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

This library did not work for me. I did not see a diff with different colors. it would be easier if we just printed out the mismatched lines IMO. Is there some section of a user guide that I have not updated?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It does seem a bit iffy. I think my macbook wasn't showing different colors, but it seems to be working right now on my thinkpad and in travis. It might have something to do with the way that the terminals handle escapes and color. This is the the way it's handled in diffmergepatch: https://github.com/sergi/go-diff/blob/master/diffmatchpatch/diff.go#L1183

If we want to keep using this library, we can either keep it like this or change to DiffToDelta, which would have an output like this instead (the numbers after =, -, and + are characters to keep, remove, or add respectively):

--- FAIL: TestGopkgtoml (0.00s)
        gopkgtoml_test.go:33: expected vs actual differs.
                =1548   -3      =72     +    unused-packages = false%0A

Copy link
Contributor

Choose a reason for hiding this comment

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

@AlexNPavel Is there any way to print out only the lines that differ for expected vs actual?
The above output is a little hard to comprehend.
The colorized diff works for me but like you pointed out its dependent on the terminal settings.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It doesn't look like the diffmergepatch library has any tools for printing lines that differ or outputting a more diff-like output. So we're kinda stuck with the above two options unless we move to a new library or make our own helper functions, which may not be that simple.

Copy link
Member

Choose a reason for hiding this comment

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

If it helps, I've been playing with this same library to get diffs out of the helm operator. Here's where I've ended up so far:

func diff(a, b string) string {
	dmp := diffmatchpatch.New()

	wSrc, wDst, warray := dmp.DiffLinesToRunes(a, b)
	diffs := dmp.DiffMainRunes(wSrc, wDst, false)
	diffs = dmp.DiffCharsToLines(diffs, warray)
	var buff bytes.Buffer
	for _, diff := range diffs {
		text := diff.Text

		switch diff.Type {
		case diffmatchpatch.DiffInsert:
			_, _ = buff.WriteString(prefixLines(text, "+"))
		case diffmatchpatch.DiffDelete:
			_, _ = buff.WriteString(prefixLines(text, "-"))
		case diffmatchpatch.DiffEqual:
			_, _ = buff.WriteString(prefixLines(text, " "))
		}
	}
	return buff.String()
}

func prefixLines(s, prefix string) string {
	var buf bytes.Buffer
	lines := strings.Split(s, "\n")
	ls := regexp.MustCompile("^")
	for _, line := range lines[:len(lines)-1] {
		buf.WriteString(ls.ReplaceAllString(line, prefix))
		buf.WriteString("\n")
	}
	return buf.String()
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@joelanford that works very well. Thanks. Since this diff work is unrelated to this PR, I'll leave this PR as is and make a new PR to change that replaces the diffs in the current tests with your methods, plus color as that can improve readability in terminals that support it. The lines based method is much easier to read, especially if you're only modifying a couple characters in a line, which can become really messy with the DiffPrettyText method.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah let's add in Joe's diff changes in a new PR.

}
}

const serviceAccountExp = `apiVersion: v1
kind: ServiceAccount
metadata:
name: app-operator
`
Binary file added test/ansible-operator/ansible-operator
Binary file not shown.
Loading