Issue #1824 Kafka: Support access logging for Kafka requests/responses #1870

manalibhutiyani · 2017-10-27T02:13:52Z

This is the first pass of supporting access.logging for Kafka policy-enforcement.
Addresses #1824
Signed-off by: Manali Bhutiyani manali@covalent.io

houndci-bot · 2017-10-27T02:13:58Z

pkg/proxy/kafka.go

+	}
+}
+
+func (r *kafkaRedirect) getDestinationInfo(dstIPPort string) accesslog.EndpointInfo {


receiver name r should be consistent with previous receiver name k for kafkaRedirect

houndci-bot · 2017-10-27T02:13:58Z

pkg/proxy/kafka.go

+
+// egressDestinationInfo returns the destination EndpointInfo for a flow
+// leaving the proxy at egress.
+func (r *kafkaRedirect) egressDestinationInfo(ipstr string, info *accesslog.EndpointInfo) {


receiver name r should be consistent with previous receiver name k for kafkaRedirect

houndci-bot · 2017-10-27T02:13:58Z

pkg/proxy/kafka.go

+//  - info.Identity
+//  - info.Labels
+//  - info.LabelsSHA256
+func (r *kafkaRedirect) fillReservedIdentity(info *accesslog.EndpointInfo, id policy.NumericIdentity) {


receiver name r should be consistent with previous receiver name k for kafkaRedirect

houndci-bot · 2017-10-27T02:13:58Z

pkg/proxy/kafka.go

+	}
+}
+
+func (r *kafkaRedirect) getSourceInfo(remoteAddr string,


receiver name r should be consistent with previous receiver name k for kafkaRedirect

houndci-bot · 2017-10-27T02:13:58Z

pkg/proxy/kafka.go

+// ingress policies are set, the ingress policy cannot determine the source
+// endpoint info based on ip address, as the ip address would be that of the
+// egress proxy i.e host.
+func (r *kafkaRedirect) getInfoFromConsumable(ipstr string,


receiver name r should be consistent with previous receiver name k for kafkaRedirect

houndci-bot · 2017-10-27T02:13:58Z

pkg/proxy/accesslog/log.go

@@ -44,6 +43,58 @@ const (
 	FieldFilePath = "file-path"
 )

+const (
+	FieldKafkaApiKey        = "kafkaApiKey"
+	FieldKafkaApiVersion    = "kafkaApiVersion"


const FieldKafkaApiVersion should be FieldKafkaAPIVersion

houndci-bot · 2017-10-27T02:13:58Z

pkg/proxy/accesslog/log.go

@@ -44,6 +43,58 @@ const (
 	FieldFilePath = "file-path"
 )

+const (
+	FieldKafkaApiKey        = "kafkaApiKey"


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

houndci-bot · 2017-10-27T02:13:59Z

pkg/kafka/request.go

@@ -54,6 +66,69 @@ func (req *RequestMessage) String() string {
 		req.kind, req.version, len(req.rawMsg), string(b))
 }

+func (req *RequestMessage) GetTopics() []string {


exported method RequestMessage.GetTopics should have comment or be unexported

houndci-bot · 2017-10-27T02:13:59Z

pkg/kafka/request.go

+	return req.version
+}
+
+func (req *RequestMessage) GetCorrelationID() int32 {


exported method RequestMessage.GetCorrelationID should have comment or be unexported

houndci-bot · 2017-10-27T02:13:59Z

pkg/kafka/error.go

+package kafka
+
+var (
+	ErrUnknown                                 = -1


exported var ErrUnknown should have comment or be unexported

ashwinp · 2017-10-27T02:34:59Z

Changes to log.go and record.go LGTM.

raybejjani

Some random style comments from me, nothing blocking (and you can ignore them too)

raybejjani · 2017-10-27T08:30:00Z

pkg/kafka/error.go

+	ErrTopicAuthorizationFailed                = 29
+	ErrGroupAuthorizationFailed                = 30
+	ErrClusterAuthorizationFailed              = 31
+	ErrInvalidTimeStamp                        = 32


Not too important. Assuming these are our internal errors (and not mapped to an external set of Error codes) you can use iota to avoid numbering them (of course, you already numbered them so I'm kinda suggesting you do double work :/)
https://golang.org/ref/spec#Iota

const ( ErrUnknown = -1 ErrNone = iota ErrOffsetOutOfRange ErrInvalidMessage )

oh, I see they're actual Kafka errors, ignore me.

Added a comment to make it more clear.

raybejjani · 2017-10-27T08:39:12Z

pkg/kafka/request.go

+	return int32(binary.BigEndian.Uint32(req.rawMsg[8:12]))
+}
+
+func (req *RequestMessage) getVersion() int16 {


This might be less confusing if named extractVersion or something different than the exported GetVersion function.

I was responsible for this naming but I agree. extract or read is better here.

Changed to extractVersion

raybejjani · 2017-10-27T08:50:43Z

pkg/proxy/accesslog/log.go

+					FieldFilePath: logPath,
+				}).Debug("Skipping writing to access log (write buffer nil)")
+				return
+			}


You could move this whole block out of the switch so it isn't repeated (I realise it was here to begin with, but since you're shuffling things around anyway).

raybejjani · 2017-10-27T08:56:56Z

pkg/proxy/accesslog/log.go

+				FieldProtocol: l.HttpRequest.Proto,
+				FieldHeader:   l.HttpRequest.Header,
+				FieldFilePath: logPath,
+			}).Debug("Logging HTTP L7 flow record")


If you prefer, you can create a logrus object outside the switch with fields common to each switch case, then use that. It would ensure some consistency as we add more cases, but it's not a big deal.

scopedLog := log.WithFields(log.Fields{ FieldType: typ, FieldVerdict: verdict, FieldCode: code, FieldFilePath: logPath, }) switch L7Type { case L7TypeHTTP: scopedLog.WithFields(log.Fields{ case specific fields }).Debug("blah") }

This was a merge rebase fallout., I think... will fix it

raybejjani · 2017-10-27T08:57:51Z

pkg/proxy/accesslog/log.go

-	}
+	switch L7type {
+	case L7TypeHTTP:
+		{


Are these {} for variable scoping? I've never seen it before but it certainly works :)

It's probably there to make C programmers feel more at home

Maybe we need some semicolons? :P

I will remove this sorry.. it is the C programmer talking!

raybejjani · 2017-10-27T09:02:09Z

pkg/proxy/kafka.go

+			log.Warn("Missing security identity in source endpoint info")
+		}
+
+	}


If you didn't want to have so much indentation, you can use a switch here:

switch { case !r.ingress: blah case err == nil && srcIdentity != 0: bloh case err == nil: bleeh }

tgraf · 2017-10-27T07:43:33Z

pkg/kafka/request.go

+}
+
+func (req *RequestMessage) GetCorrelationID() int32 {
+	return int32(binary.BigEndian.Uint32(req.rawMsg[8:12]))


The Kafka parser only guarantees that the first 8 bytes have been read into rawMsg. You need to check len() of rawMsg in ReadRequest() to be at least 12 bytes to avoid out of bounds accesses.

Adding
if len(req.rawMsg) < 12 { return nil, fmt.Errorf("unable to read full request") }

in ReadRequest

tgraf · 2017-10-27T07:44:26Z

pkg/kafka/request.go

@@ -54,6 +66,69 @@ func (req *RequestMessage) String() string {
 		req.kind, req.version, len(req.rawMsg), string(b))
 }

+func (req *RequestMessage) GetTopics() []string {


tgraf · 2017-10-27T07:46:04Z

pkg/kafka/request.go

@@ -121,11 +192,5 @@ func ReadRequest(reader io.Reader) (*RequestMessage, error) {
 		req.request, err = proto.ReadOffsetFetchReq(buf)
 	}

-	if err != nil {


Why are you removing this error check?

tgraf · 2017-10-27T07:48:50Z

pkg/proxy/accesslog/log.go

@@ -44,6 +43,58 @@ const (
 	FieldFilePath = "file-path"
 )

+const (
+	FieldKafkaApiKey        = "kafkaApiKey"


Continue in the above const() section?

I wanted to keep it separate for "Kafka log record messages" as opposed the shared Log record fields for L7 messages.

tgraf · 2017-10-27T08:00:43Z

pkg/policy/api/selector.go

@@ -154,7 +152,6 @@ func (n *EndpointSelector) Matches(lblsToMatch k8sLbls.Labels) bool {
 		// FIXME: Omit this error or throw it to the caller?
 		// We are doing the verification in the ParseEndpointSelector but
 		// don't make sure the user can modify the current labels.
-		log.WithError(err).WithField(logfields.EndpointLabelSelector, logfields.Repr(n)).Error("unable to match label selector in selector")


Can we remove this in a separate PR with proper explanation why it is safe? If we remove the error the FIXME needs to go as well.

I will rebase this, this was a fallout of the circular dependency I was getting... @raybejjani has fixed this, I should be able to undo this part.

tgraf · 2017-10-27T08:45:06Z

pkg/proxy/kafka.go

+	}
+}
+
+func (r *kafkaRedirect) getSourceInfo(remoteAddr string,


Can we share this function with HTTP? It looks identical

tgraf · 2017-10-27T08:45:24Z

pkg/proxy/kafka.go

+//  - info.Identity
+//  - info.Labels
+//  - info.LabelsSHA256
+func (r *kafkaRedirect) fillReservedIdentity(info *accesslog.EndpointInfo, id policy.NumericIdentity) {


tgraf · 2017-10-27T09:04:22Z

pkg/proxy/kafka.go

@@ -185,14 +373,29 @@ func (k *kafkaRedirect) handleRequest(pair *connectionPair, req *kafka.RequestMe
 			fieldID:  pair.String(),
 			"source": addr.String(),
 		}).WithError(err).Error("Unable lookup original destination")
+		record.Info = fmt.Sprintf("Invalid message format: %s", err)
+		accesslog.Log(record, accesslog.TypeRequest, accesslog.VerdictError,


error should be: unable to lookup original destination

tgraf · 2017-10-27T09:06:10Z

pkg/proxy/kafka.go

@@ -205,6 +408,11 @@ func (k *kafkaRedirect) handleRequest(pair *connectionPair, req *kafka.RequestMe
 		return
 	}

+	// log valid request
+	record.Timestamp = time.Now().UTC().Format(time.RFC3339Nano)
+	accesslog.Log(record, accesslog.TypeRequest, accesslog.VerdictForwarded,


This is too early, dialing the original destination can still fail here

tgraf · 2017-10-27T09:08:14Z

pkg/proxy/kafka.go

+
+	// log valid response
+	record.Timestamp = time.Now().UTC().Format(time.RFC3339Nano)
+	accesslog.Log(record, accesslog.TypeResponse, accesslog.VerdictForwarded,


Unlike with HTTP, the responses are parsed as well so we can log them there. See handleResponseConnection()

michi-covalent · 2017-10-28T20:46:14Z

@manalibhutiyani could you resolve the conflicts?

manalibhutiyani · 2017-10-28T20:47:31Z

@michi-covalent : yes, I will address the review comments and rebase the branch.

houndci-bot · 2017-10-30T01:31:52Z

pkg/proxy/accesslog/log.go

@@ -45,6 +45,58 @@ const (
 	FieldFilePath = logfields.Path
 )

+const (
+	FieldKafkaApiKey        = "kafkaApiKey"


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

aanm · 2017-10-30T10:33:59Z

pkg/proxy/accesslog/log.go

+			}
+
+			/*
+			 *	Log multiple entries for multiple Kafka topics in a single


Add a FIXME tag here as well

This is not really.a FIXME. The GH issue is to point out why we are logging multiple access logs for a single Kafka request

aanm · 2017-10-30T10:34:45Z

pkg/proxy/accesslog/record.go

-	// Internal
-	Request *http.Request `json:"-"`
+	// Internal HTTP request
+	HttpRequest *http.Request `json:"-"`


@manalibhutiyani ^

aanm · 2017-10-30T10:35:41Z

pkg/proxy/kafka.go

@@ -47,6 +52,19 @@ type kafkaRedirect struct {
 	socket   *proxySocket
 }

+/*
+id       string


What is this? leftovers?

aanm · 2017-10-30T10:37:42Z

pkg/proxy/kafka.go

+// leaving the proxy at egress.
+func (r *kafkaRedirect) egressDestinationInfo(ipstr string, info *accesslog.EndpointInfo) {
+	ip := net.ParseIP(ipstr)
+	if ip != nil {


To avoid indentation write if ip == nil { return }

Manali: @aanm goFmt automatically formats this to indented values for me.

@manalibhutiyani if you do if ip == nil { return } the following part of the code won't have an extra unnecessary indentation.

aanm · 2017-10-30T10:42:21Z

pkg/proxy/kafka.go

+				c := addressing.DeriveCiliumIPv4(ip)
+				ep := endpointmanager.LookupIPv4(c.String())
+				if ep != nil {
+					info.ID = uint64(ep.ID)


you're missing endpoint.RLock() and endpoint.RUnlock()

This was missing from the beginning (before Kafka). Added.

aanm · 2017-10-30T10:44:18Z

pkg/proxy/kafka.go

+func (r *kafkaRedirect) getDestinationInfo(dstIPPort string) accesslog.EndpointInfo {
+	info := accesslog.EndpointInfo{}
+	ipstr, port, err := net.SplitHostPort(dstIPPort)
+	if err == nil {


If dstIPPort returns an error, why continue the execution of this function?

Add debug log to show the error in case we ever it this.

To avoid indentation write if ip == nil { return }

Manali: @aanm Not sure why I cannot comment here, but we need to log even if we fail to Destination Info. This just means the destination info will not be present in access log.

aanm · 2017-10-30T10:44:24Z

pkg/proxy/kafka.go

+	ipstr, port, err := net.SplitHostPort(dstIPPort)
+	if err == nil {
+		p, err := strconv.ParseUint(port, 10, 16)
+		if err == nil {


Same here

@aanm :answered above

houndci-bot · 2017-10-30T18:27:47Z

pkg/policy/api/rule.go

+// KafkaMaxTopicVal is the maximum value of supported API Keys in KafkaAPIKeyMap
+// KafkaReverseAPIKeyMap
+const (
+	KafkaMaxApiKeyVal = 33


const KafkaMaxApiKeyVal should be KafkaMaxAPIKeyVal

houndci-bot · 2017-10-30T18:27:47Z

pkg/policy/api/rule.go

+// KafkaReverseApiKeyMap is the map of all allowed kafka API keys
+// with the key values.
+// Reference: https://kafka.apache.org/protocol#protocol_api_keys
+var KafkaReverseApiKeyMap = map[int16]string{


var KafkaReverseApiKeyMap should be KafkaReverseAPIKeyMap

tgraf · 2017-10-30T20:04:13Z

pkg/kafka/request.go

@@ -99,7 +173,10 @@ func ReadRequest(reader io.Reader) (*RequestMessage, error) {
 		return nil, err
 	}

-	req.version = req.getVersion()
+	if len(req.rawMsg) < 12 {
+		return nil, fmt.Errorf("unable to read full request")


This needs a more descriptive error message, something like:
unexpected end of request (length < 12 bytes)

tgraf · 2017-10-30T20:04:34Z

pkg/kafka/request.go

+	}

This style change seems unwanted

tgraf · 2017-10-30T20:06:31Z

pkg/proxy/accesslog/log.go

+	if apiKey < 0 || apiKey > api.KafkaMaxAPIKeyVal {
+		log.WithFields(log.Fields{
+			FieldFilePath: logPath,
+		}).Error("Invalid Kafka Api Key")


Why are we not logging this? If it was observed on the wire then we should log it in the access log. I understand that this is protecting the map access. What I would do here instead is create a function which checks whether there is a hit in the map and if no hit is found, encode the apiKey as number in string representation.

I personally think that since this has already gone through canAccess by the time we actually log it, the apiKey is already checked for correct values, so this failure will not be hit Imo, please correct me if I am wrong. Let me re-organise and see how I can change the code here.

I see that it can be hit before canAccess. Ignore my earlier comment

changed as per discussion offline.

tgraf · 2017-10-30T20:09:17Z

pkg/proxy/accesslog/log.go

+	}
+
+	/*
+	 *	Log multiple entries for multiple Kafka topics in a single


You are in C land here ;-)

changed the comment style. :)

tgraf · 2017-10-30T20:10:47Z

pkg/proxy/accesslog/record.go


 	// APIKey for Kafka message
 	// Reference: https://kafka.apache.org/protocol#protocol_api_keys
-	APIKey int16
+	APIKey string

 	// CorrelationID is a user-supplied integer value that will be passed
 	// back with the response
 	CorrelationID int32



tgraf · 2017-10-30T20:12:20Z

pkg/kafka/request.go

@@ -54,6 +68,70 @@ func (req *RequestMessage) String() string {
 		req.kind, req.version, len(req.rawMsg), string(b))
 }

+// GetTopics returns the Kafka request list of topics
+func (req *RequestMessage) GetTopics() []string {
+	switch val := req.request.(type) {


I think req.request can be nil here

Added a check before dereferencing.
if req.request == nil { return nil }

houndci-bot · 2017-10-30T21:04:23Z

pkg/proxy/accesslog/log.go

+func apiKeyToString(apiKey int16) string {
+	if key, ok := api.KafkaReverseAPIKeyMap[apiKey]; ok {
+		return key
+	} else {


if block ends with a return statement, so drop this else and outdent its block (move short variable declaration to its own line if necessary)

tgraf · 2017-10-30T20:50:22Z

pkg/proxy/kafka.go

+
+// egressDestinationInfo returns the destination EndpointInfo for a flow
+// leaving the proxy at egress.
+func (k *kafkaRedirect) egressDestinationInfo(ipstr string, info *accesslog.EndpointInfo) {


This function can be removed, EgressDestinationInfo() in proxy.go can be made private and can be called directly. The *kafkaRedirect context is not required

tgraf · 2017-10-30T20:50:40Z

pkg/proxy/kafka.go

+}
+
+// getDestinationInfo returns the destination EndpointInfo.
+func (k *kafkaRedirect) getDestinationInfo(dstIPPort string) accesslog.EndpointInfo {


getDestinationInfo needs redirect information from individual proxy redirects

tgraf · 2017-10-30T21:04:05Z

pkg/proxy/kafka.go

 	addr := pair.rx.conn.RemoteAddr()
 	if addr == nil {
 		scopedLog.Warn("RemoteAddr() is nil")
+		record.Info = fmt.Sprint("RemoteAddr() is nil")


You can fill record first and then print the warning like this:

scopedLog.Warn(record.Info)

tgraf · 2017-10-30T21:04:43Z

pkg/proxy/kafka.go

@@ -174,15 +231,35 @@ func (k *kafkaRedirect) handleRequest(pair *connectionPair, req *kafka.RequestMe
 	srcIdentity, dstIPPort, err := k.conf.lookupNewDest(addr.String(), k.conf.listenPort)
 	if err != nil {
 		log.WithField("source", addr.String()).WithError(err).Error("Unable lookup original destination")
+		record.Info = fmt.Sprintf("Invalid message format: %s", err)


Invalid message format doesn't make sense as error here. Use the Unable to lookup original destination error

tgraf · 2017-10-30T21:06:37Z

pkg/proxy/oxyproxy.go

-	}
+// fillReservedIdentity resolves the labels of the specified identity if known
+// locally and fills in the info member fields.
+func (r *OxyRedirect) fillReservedIdentity(info *accesslog.EndpointInfo, id policy.NumericIdentity) {


This function can be removed and the proxy.go version can be called directly

tgraf · 2017-10-30T21:06:42Z

pkg/proxy/oxyproxy.go

-	return info
+// egressDestinationInfo returns the destination EndpointInfo for a flow
+// leaving the proxy at egress.
+func (r *OxyRedirect) egressDestinationInfo(ipstr string, info *accesslog.EndpointInfo) {


tgraf · 2017-10-30T21:07:15Z

pkg/proxy/proxy.go

@@ -142,6 +151,181 @@ func (p *Proxy) allocatePort() (uint16, error) {

 var gcOnce sync.Once

+// LocalEndpointInfo fills the access log with the local endpoint info.
+func LocalEndpointInfo(info *accesslog.EndpointInfo, source ProxySource,


All of these functions can be marked private

LocalEndpointInfo needs redirect info for individual Kafka redirect as well as Oxyproxy,

It's still weird to have proxy.go call an interface which requires proxy.go functionality itself again.

Looking at LocalEndpointInfo(), it requires ProxySource and the ID of the endpoint it is proxying for. The endpoint ID can be retrieved via the source using source.GetID()

Instead of writing into kafka and back into proxy, I'd rather add getSource() to Redirect interface which then just has to return the source

tgraf · 2017-10-30T21:07:35Z

pkg/proxy/source_mock_test.go

-func (m *proxySourceMocker) RLock()   { m.RLock() }
-func (m *proxySourceMocker) RUnlock() { m.RUnlock() }
+func (m *proxySourceMocker) RLock()   { m.RWMutex.RLock() }
+func (m *proxySourceMocker) RUnlock() { m.RWMutex.RUnlock() }


This change seems unnecessary.

As per @aanm this was needed for one of the tests failing. ?

@tgraf it is necessary, otherwise the RLock() was a recursive function. :)

houndci-bot · 2017-10-30T22:58:13Z

pkg/proxy/kafka.go

@@ -159,30 +160,82 @@ func (k *kafkaRedirect) canAccess(req *kafka.RequestMessage, numIdentity policy.
 	return req.MatchesRule(rules.Kafka)
 }

+func (r *kafkaRedirect) getSource() ProxySource {


receiver name r should be consistent with previous receiver name k for kafkaRedirect

tgraf · 2017-10-30T23:45:13Z

pkg/proxy/proxy.go

+}
+
+// GetDestinationInfo returns the destination EndpointInfo.
+func GetDestinationInfo(dstIPPort string, r Redirect, ingress bool) accesslog.EndpointInfo {


This can still be made private

sure.. my concepts on private functions in a golang package are a bit shaky :) Will do

tgraf · 2017-10-30T23:45:27Z

pkg/proxy/proxy.go

+
+// GetSourceInfo resolves source information
+// using source identity and fills the access log.
+func GetSourceInfo(RemoteAddr string,


This can still be made private

sure. my concepts on private functions in a golang package are a bit shaky :) Will do

…enforcement. Signed-off by: Manali Bhutiyani manali@covalent.io

Addresses #1824 Signed-off by: Manali Bhutiyani manali@covalent.io

…logging code Addresses #1824 Signed-off by: Manali Bhutiyani manali@covalent.io

…ests Addresses #1824 Signed-off by: Manali Bhutiyani manali@covalent.io

Addresses #1824 Signed-off by: Manali Bhutiyani manali@covalent.io

… to access log Addresses #1824 Signed-off by: Manali Bhutiyani manali@covalent.io

…vate Addresses #1824 Signed-off by: Manali Bhutiyani manali@covalent.io

Addresses #1824 Signed-off by: Manali Bhutiyani manali@covalent.io

raybejjani

@manalibhutiyani Thanks for the fixups!

aanm · 2017-10-31T15:49:11Z

@manalibhutiyani The remaining changes can be fixed on a following up PR

This is the first pass of supporting access.logging for Kafka policy enforcement. Fixes: #1824 Backports: 95b7115 Signed-off by: Manali Bhutiyani manali@covalent.io

manalibhutiyani requested review from tgraf and a team as code owners October 27, 2017 02:13

houndci-bot reviewed Oct 27, 2017

View reviewed changes

manalibhutiyani requested review from ashwinp and michi-covalent October 27, 2017 02:17

raybejjani reviewed Oct 27, 2017

View reviewed changes

tgraf requested changes Oct 27, 2017

View reviewed changes

manalibhutiyani requested review from rlenglet and a team October 27, 2017 17:48

manalibhutiyani force-pushed the kafka-access-log branch from 63fec2b to 728f58d Compare October 30, 2017 01:31

houndci-bot reviewed Oct 30, 2017

View reviewed changes

aanm requested changes Oct 30, 2017

View reviewed changes

aanm mentioned this pull request Oct 30, 2017

Generalize access logging (rebased) #1889

Merged

houndci-bot reviewed Oct 30, 2017

View reviewed changes

manalibhutiyani added pending review release-note/major This PR introduces major new functionality to Cilium. labels Oct 30, 2017

manalibhutiyani force-pushed the kafka-access-log branch 2 times, most recently from 97cf641 to c9c8aa2 Compare October 30, 2017 19:58

tgraf requested changes Oct 30, 2017

View reviewed changes

houndci-bot reviewed Oct 30, 2017

View reviewed changes

tgraf requested changes Oct 30, 2017

View reviewed changes

houndci-bot reviewed Oct 30, 2017

View reviewed changes

tgraf requested changes Oct 30, 2017

View reviewed changes

manalibhutiyani force-pushed the kafka-access-log branch from 1c13523 to 649e337 Compare October 31, 2017 01:23

This is the first pass of supporting access.logging for Kafka policy …

075d523

…enforcement. Signed-off by: Manali Bhutiyani manali@covalent.io

Manali Bhutiyani added 8 commits October 30, 2017 18:45

Issue #1824 Cleanup

79b7c2e

Addresses #1824 Signed-off by: Manali Bhutiyani manali@covalent.io

Issue #1824 Decouple and remove duplication of HTTP and Kafka access …

b2bbdf8

…logging code Addresses #1824 Signed-off by: Manali Bhutiyani manali@covalent.io

Issue #1824 Add more error reporting in the access log for Kafka requ…

4fd2d00

…ests Addresses #1824 Signed-off by: Manali Bhutiyani manali@covalent.io

Issue #1824 Fix a null dereference

ec6e55c

Addresses #1824 Signed-off by: Manali Bhutiyani manali@covalent.io

Issue #1824 Add checks/lookup function for Kafka apikey while logging…

0244666

… to access log Addresses #1824 Signed-off by: Manali Bhutiyani manali@covalent.io

Issue #1824 Address review comments of making the Proxy functions pri…

d56e1bf

…vate Addresses #1824 Signed-off by: Manali Bhutiyani manali@covalent.io

Issue #1824 make getDestinationInfo and getSourceInfo private

784e8d5

Addresses #1824 Signed-off by: Manali Bhutiyani manali@covalent.io

Issue #1824 Address review comments

7faa7cb

Addresses #1824 Signed-off by: Manali Bhutiyani manali@covalent.io

manalibhutiyani force-pushed the kafka-access-log branch from 649e337 to 7faa7cb Compare October 31, 2017 01:46

manalibhutiyani requested review from a team October 31, 2017 05:57

raybejjani approved these changes Oct 31, 2017

View reviewed changes

tgraf approved these changes Oct 31, 2017

View reviewed changes

aanm approved these changes Oct 31, 2017

View reviewed changes

tgraf merged commit 95b7115 into master Oct 31, 2017

tgraf deleted the kafka-access-log branch October 31, 2017 15:50

Issue #1824 Kafka: Support access logging for Kafka requests/responses #1870

Issue #1824 Kafka: Support access logging for Kafka requests/responses #1870

Conversation

manalibhutiyani commented Oct 27, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ashwinp commented Oct 27, 2017

raybejjani left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

manalibhutiyani Oct 30, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

michi-covalent commented Oct 28, 2017

manalibhutiyani commented Oct 28, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aanm Oct 30, 2017 • edited by manalibhutiyani Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

manalibhutiyani Oct 30, 2017 • edited Loading

Choose a reason for hiding this comment

aanm Oct 30, 2017 • edited by manalibhutiyani Loading

Choose a reason for hiding this comment

aanm Oct 30, 2017 • edited by manalibhutiyani Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

manalibhutiyani Oct 30, 2017 •

edited

Loading

aanm Oct 30, 2017 •

edited by manalibhutiyani

Loading

manalibhutiyani Oct 30, 2017 •

edited

Loading

aanm Oct 30, 2017 •

edited by manalibhutiyani

Loading

aanm Oct 30, 2017 •

edited by manalibhutiyani

Loading