cmd: policy trace src/dst eID, secid, k8s-pod options by ianvernon · Pull Request #1124 · cilium/cilium

ianvernon · 2017-07-14T21:44:46Z

Add capability to pass in a source / destination endpoint, k8s pod, and security identities to cilium policy trace.

Signed-off by: Ian Vernon ian@covalent.io

Partially addresses #1005

houndci-bot · 2017-07-14T21:44:50Z

var dstSecurityIdLabels should be dstSecurityIDLabels

houndci-bot · 2017-07-14T21:44:50Z

var srcSecurityIdLabels should be srcSecurityIDLabels

houndci-bot · 2017-07-14T21:44:50Z

const defaultSecurityId should be defaultSecurityID

houndci-bot · 2017-07-21T17:38:52Z

func parameter epId should be epID

houndci-bot · 2017-07-24T18:55:51Z

exported method BoolOptions.GetModel should have comment or be unexported

houndci-bot · 2017-07-24T18:55:52Z

exported method BoolOptions.GetMutableModel should have comment or be unexported

houndci-bot · 2017-07-24T18:55:52Z

exported method BoolOptions.GetImmutableModel should have comment or be unexported

houndci-bot · 2017-07-25T02:35:26Z

 	}

-	return inLabels, nil
+	secId := p.GetAnnotations()["cilium-identity"]


var secId should be secID

houndci-bot · 2017-07-25T02:35:26Z

-		if resp == nil {
-			return nil, fmt.Errorf("ID %s not found", id)
-		}
+func getSecIdFromK8s(podName string) (string, error) {


func getSecIdFromK8s should be getSecIDFromK8s

houndci-bot · 2017-07-25T02:35:26Z

-func parseAllowedSlice(slice []string) ([]string, error) {
-	inLabels := []string{}
-	id := ""
+func appendEpLabelsToSlice(epId string, labelSlice []string) []string {


func parameter epId should be epID

houndci-bot · 2017-07-27T17:31:34Z

 	return d.conf.Opts.IsEnabled(endpoint.OptionDebug)
 }

+func (d *Daemon) AnnotateEndpoint(e *endpoint.Endpoint, annotationKey, annotationValue string) {


exported method Daemon.AnnotateEndpoint should have comment or be unexported

houndci-bot · 2017-07-27T17:31:34Z

+		return "", fmt.Errorf("unable to get pod %s in namespace %s", namespace, pod)
+	}
+
+	secId := p.GetAnnotations()["cilium-identity"]


var secId should be secID

houndci-bot · 2017-07-27T17:31:34Z

+			if err != nil {
+				Fatalf("Cannot get security id from k8s pod name: %s", err)
+			}
+			convertedId, err := strconv.ParseInt(id, 0, 64)


var convertedId should be convertedID

houndci-bot · 2017-07-27T17:31:34Z

+			if err != nil {
+				Fatalf("Cannot get security id from k8s pod name: %s", err)
+			}
+			convertedId, err := strconv.ParseInt(id, 0, 64)


var convertedId should be convertedID

houndci-bot · 2017-08-01T22:39:22Z

var secId should be secID

houndci-bot · 2017-08-01T22:39:22Z

var secId should be secID

ianvernon · 2017-08-06T21:06:14Z

Between commit 5dfd8b1 and commit 364d72ec43ecb0bd6546bc9a3d25dc402ac80b4f, the adding of the annotation failed in the K8s case:

20:47:47 [K8s multi node Tests] time="2017-08-06T20:47:41Z" level=error msg="k8s: unable to update pod default:app2-4193432878-blhq1 with \"cilium-identity\" annotation: pods \"app2-4193432878-blhq1\" is forbidden: unexpected operation UPDATE, retrying..." 
20:47:47 [K8s multi node Tests] time="2017-08-06T20:47:41Z" level=error msg="k8s: unable to update pod default:app3-3595226997-mq28v with \"cilium-identity\" annotation: pods \"app3-3595226997-mq28v\" is forbidden: unexpected operation UPDATE, retrying..." 
20:47:47 [K8s multi node Tests] time="2017-08-06T20:47:41Z" level=error msg="k8s: unable to update pod default:app1-2242192615-84jwr with \"cilium-identity\" annotation: pods \"app1-2242192615-84jwr\" is forbidden: unexpected operation UPDATE, retrying..." 
20:47:47 [K8s multi node Tests] time="2017-08-06T20:47:41Z" level=error msg="k8s: unable to update pod default:app1-2242192615-wrdqw with \"cilium-identity\" annotation: pods \"app1-2242192615-wrdqw\" is forbidden: unexpected operation UPDATE, retrying..."

The only change was to add nodes to the list of objects that allow for updates in the RBAC YAML file. I'm not sure why this would happen? Will investigate...

houndci-bot · 2017-08-08T04:15:30Z

exported const CiliumIdentityAnnotation should have comment (or a comment on this block) or be unexported

Add capability to pass in a source / destination endpoint, k8s pod, and security identities to `cilium policy trace`. Signed-off by: Ian Vernon <ian@covalent.io>

ianvernon · 2017-08-08T15:32:23Z

-	inLabels := []string{}
-	id := ""
+// Returns the labels for security identity ID and an error if the labels cannot be retrieved.
+func getLabelsFromIdentity(ID int64) ([]string, error) {


I am going to refactor this function into pkg/client/identity.go

houndci-bot reviewed Jul 14, 2017

View reviewed changes

ianvernon force-pushed the 1005-cilium-policy-trace-usability branch 8 times, most recently from 8496945 to 6ece0a0 Compare July 21, 2017 00:19

houndci-bot reviewed Jul 21, 2017

View reviewed changes

Comment thread cilium/cmd/policy_trace.go Outdated

Copy link
Copy Markdown

houndci-bot Jul 21, 2017

Choose a reason for hiding this comment

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

func parameter epId should be epID

ianvernon force-pushed the 1005-cilium-policy-trace-usability branch 2 times, most recently from 6e807bb to d61a609 Compare July 24, 2017 18:05

houndci-bot reviewed Jul 24, 2017

View reviewed changes

ianvernon force-pushed the 1005-cilium-policy-trace-usability branch from ea7dd3c to 61f3b51 Compare July 25, 2017 02:35

houndci-bot reviewed Jul 25, 2017

View reviewed changes

ianvernon force-pushed the 1005-cilium-policy-trace-usability branch from 0607376 to e5a3e38 Compare July 27, 2017 17:31

houndci-bot reviewed Jul 27, 2017

View reviewed changes

ianvernon force-pushed the 1005-cilium-policy-trace-usability branch from e5a3e38 to d30c11a Compare August 1, 2017 22:39

houndci-bot reviewed Aug 1, 2017

View reviewed changes

Comment thread cilium/cmd/policy_trace.go Outdated

Copy link
Copy Markdown

houndci-bot Aug 1, 2017

Choose a reason for hiding this comment

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

var secId should be secID

houndci-bot reviewed Aug 1, 2017

View reviewed changes

Comment thread cilium/cmd/policy_trace.go Outdated

Copy link
Copy Markdown

houndci-bot Aug 1, 2017

Choose a reason for hiding this comment

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

var secId should be secID

ianvernon force-pushed the 1005-cilium-policy-trace-usability branch from ede56b3 to 8775e5b Compare August 2, 2017 18:51

ianvernon force-pushed the 1005-cilium-policy-trace-usability branch 2 times, most recently from 72c8e87 to fa9e1fb Compare August 8, 2017 04:15

houndci-bot reviewed Aug 8, 2017

View reviewed changes

ianvernon force-pushed the 1005-cilium-policy-trace-usability branch from fa9e1fb to 9f992ff Compare August 8, 2017 04:17

ianvernon changed the title ~~WIP - cmd: policy trace src / dst eID and secid options~~ cmd: policy trace src/dst eID, secid, k8s-pod options Aug 8, 2017

ianvernon added the wip label Aug 8, 2017

cmd: policy trace src/dst eID, secid, k8s-pod options

0285330

Add capability to pass in a source / destination endpoint, k8s pod, and security identities to `cilium policy trace`. Signed-off by: Ian Vernon <ian@covalent.io>

ianvernon force-pushed the 1005-cilium-policy-trace-usability branch from abca452 to 0285330 Compare August 8, 2017 04:46

ianvernon added pending-review and removed wip labels Aug 8, 2017

ianvernon requested a review from tgraf August 8, 2017 15:22

ianvernon commented Aug 8, 2017

View reviewed changes

tgraf approved these changes Aug 8, 2017

View reviewed changes

tgraf merged commit 31266bd into master Aug 8, 2017

tgraf deleted the 1005-cilium-policy-trace-usability branch August 8, 2017 15:34

Conversation

ianvernon commented Jul 14, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ianvernon commented Aug 6, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

ianvernon commented Jul 14, 2017 •

edited

Loading