feat(controller): Add audit logs to workflows. Fixes #1769 #1930

amarrella · 2020-01-09T16:45:37Z

Checklist:

Either (a) I've created an enhancement proposal and discussed it with the community, (b) this is a bug fix, or (c) this does not need to be in the release notes. -> it was proposed by one of the maintainers in Add audit logging for Workflow resources #1769
The title of the PR is (a) conventional, (b) states what changed, and (c) suffixes the related issues number. E.g. "fix(controller): Updates such and such. Fixes #1234". Why? for the release notes.
Optional. I've added My organization is added to the README.
I've signed the CLA and required builds are green.

I pretty much ported the argo-cd implementation in https://github.com/argoproj/argo-cd/blob/0675ff2fb25fe97856171230f150e6ba5a2563d5/util/argo/audit_logger.go

util/argo/audit_logger.go

workflow/controller/operator.go

util/argo/audit_logger.go

codecov · 2020-01-09T16:56:30Z

Codecov Report

Merging #1930 into master will decrease coverage by 0.13%.
The diff coverage is 8.06%.

@@            Coverage Diff            @@
##           master   #1930      +/-   ##
=========================================
- Coverage    9.02%   8.89%   -0.14%     
=========================================
  Files          53      61       +8     
  Lines       33698   34083     +385     
=========================================
- Hits         3042    3030      -12     
- Misses      30267   30662     +395     
- Partials      389     391       +2

Impacted Files	Coverage Δ
persist/sqldb/null_workflow_archive.go	`0% <ø> (ø)`
pkg/apis/workflow/v1alpha1/openapi_generated.go	`0% <0%> (ø)`	⬆️
persist/sqldb/sqldb.go	`0% <0%> (ø)`
...ersist/sqldb/explosive_offload_node_status_repo.go	`0% <0%> (ø)`
pkg/apis/workflow/v1alpha1/generated.pb.go	`0.45% <0%> (-0.01%)`	⬇️
cmd/server/auth/gatekeeper.go	`30.5% <0%> (+0.5%)`	⬆️
persist/sqldb/migrate.go	`0% <0%> (ø)`
pkg/apis/workflow/v1alpha1/workflow_types.go	`7.04% <0%> (-0.2%)`	⬇️
persist/sqldb/workflow_archive.go	`0% <0%> (ø)`
persist/sqldb/backfill_cluster_name.go	`0% <0%> (ø)`
... and 19 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update de38d56...ac4d732. Read the comment docs.

alexec

This is looking really good so far. I've added a few comments, but I think they're mostly minor.

manifests/cluster-install/workflow-controller-rbac/workflow-controller-clusterrole.yaml

alexec · 2020-01-21T16:31:27Z

test/e2e/expectedfailures/failed-step-event.yaml

+  templates:
+  - name: exit
+    container:
+      image: docker/whalesay:latest


thank you for using this image - makes for faster tests

alexec · 2020-01-21T16:32:03Z

test/e2e/fixtures/then.go

@@ -67,6 +70,23 @@ func (t *Then) ExpectWorkflowList(listOptions metav1.ListOptions, block func(t *
 	return t
 }

+func (t *Then) ExpectAuditEvents(block func(*testing.T, *apiv1.EventList)) *Then {


alexec · 2020-01-21T16:32:32Z

test/e2e/functional_test.go

@@ -93,6 +97,69 @@ func (s *FunctionalSuite) TestFastFailOnPodTermination() {
 		})
 }

+func (s *FunctionalSuite) TestEventOnNodeFail() {


might be good to state what this test is meant to do

test/e2e/functional_test.go

util/argo/audit_logger.go

workflow/controller/operator_test.go

alexec

OK. I think you need to do just one more thing before we merge, run make lint. The CI listing is currently broken on master

amarrella · 2020-01-22T16:51:51Z

@alexec I just ran it and nothing happened. My golangci-lint version is

golangci-lint has version 1.23.1 built from 567904e on 2020-01-20T07:57:48Z

This is the output:

❯ make lint                                                                                                                                                                         ~/go/src/github.com/argoproj/argo
# Lint Go files
golangci-lint run --fix --verbose
INFO [config_reader] Config search paths: [./ /Users/amarrella/go/src/github.com/argoproj/argo /Users/amarrella/go/src/github.com/argoproj /Users/amarrella/go/src/github.com /Users/amarrella/go/src /Users/amarrella/go /Users/amarrella /Users /]
INFO [config_reader] Used config file .golangci.yml
INFO [lintersdb] Active 11 linters: [deadcode errcheck goimports gosimple govet ineffassign staticcheck structcheck typecheck unused varcheck]
INFO [lintersdb] Active 11 linters: [deadcode errcheck goimports gosimple govet ineffassign staticcheck structcheck typecheck unused varcheck]
INFO [loader] Go packages loading at mode 575 (exports_file|files|name|types_sizes|compiled_files|deps|imports) took 7.486166956s
INFO [runner/filename_unadjuster] Pre-built 0 adjustments in 173.551976ms
INFO [runner/unused/goanalysis] analyzers took 17.777373207s with top 10 stages: buildssa: 16.830737181s, U1000: 946.636026ms
INFO [runner/goanalysis_metalinter/goanalysis] analyzers took 2m13.29470776s with top 10 stages: buildssa: 1m14.9986216s, goimports: 8.507892103s, inspect: 3.881872092s, fact_deprecated: 3.356206081s, ctrlflow: 2.915250939s, printf: 2.649455586s, ineffassign: 2.51055655s, fact_purity: 1.685895698s, isgenerated: 1.182617992s, varcheck: 1.149710809s
INFO [runner/skip dirs] Skipped 2 issues from dir pkg/client/clientset/versioned/scheme by pattern pkg/client
INFO [runner/skip dirs] Skipped 6 issues from dir pkg/client/listers/workflow/v1alpha1 by pattern pkg/client
INFO [runner/skip dirs] Skipped 8 issues from dir pkg/client/clientset/versioned/typed/workflow/v1alpha1 by pattern pkg/client
INFO [runner/skip dirs] Skipped 2 issues from dir examples/example-golang by pattern (^|/)examples($|/)
INFO [runner/skip dirs] Skipped 6 issues from dir pkg/client/informers/externalversions/workflow/v1alpha1 by pattern pkg/client
INFO [runner/skip dirs] Skipped 4 issues from dir pkg/client/informers/externalversions by pattern pkg/client
INFO [runner/skip dirs] Skipped 7 issues from dir pkg/client/clientset/versioned/fake by pattern pkg/client
INFO [runner/skip dirs] Skipped 8 issues from dir pkg/client/clientset/versioned/typed/workflow/v1alpha1/fake by pattern pkg/client
INFO [runner/skip dirs] Skipped 2 issues from dir pkg/client/clientset/versioned by pattern pkg/client
INFO [runner/skip dirs] Skipped 2 issues from dir pkg/client/informers/externalversions/internalinterfaces by pattern pkg/client
INFO [runner] Issues before processing: 106, after processing: 0
INFO [runner] Processors filtering stat (out/in): exclude: 2/25, exclude-rules: 2/2, nolint: 0/2, filename_unadjuster: 106/106, path_prettifier: 106/106, cgo: 106/106, skip_files: 102/106, identifier_marker: 25/25, skip_dirs: 55/102, autogenerated_exclude: 25/55
INFO [runner] processing took 4.441321ms with stages: path_prettifier: 1.585795ms, autogenerated_exclude: 1.000538ms, exclude: 704.18µs, nolint: 445.513µs, identifier_marker: 327.313µs, skip_dirs: 323.097µs, skip_files: 27.718µs, cgo: 17.069µs, filename_unadjuster: 7.165µs, max_same_issues: 956ns, uniq_by_line: 416ns, source_code: 375ns, max_from_linter: 334ns, diff: 257ns, exclude-rules: 237ns, max_per_file_from_linter: 208ns, path_shortener: 150ns
INFO [runner] linters took 18.625441178s with stages: goanalysis_metalinter: 13.588482523s, unused: 5.032434385s
INFO fixer took 0s with no stages
INFO File cache stats: 0 entries of total size 0B
INFO Memory: 251 samples, avg is 1866.5MB, max is 3433.7MB
INFO Execution took 26.305625645s
# Lint UI files
yarn --cwd ui lint
yarn run v1.21.1
$ tslint --fix -p ./src/app
✨  Done in 3.90s.

alexec · 2020-01-22T16:55:42Z

Merged! This will be in v2.5

amarrella · 2020-01-22T16:56:28Z

Thanks @alexec ! :)

Add audit logs to workflows

273a560

alexec reviewed Jan 9, 2020

View reviewed changes

util/argo/audit_logger.go Show resolved Hide resolved

alexec self-assigned this Jan 9, 2020

alexec reviewed Jan 9, 2020

View reviewed changes

util/argo/audit_logger.go Outdated Show resolved Hide resolved

workflow/controller/operator.go Outdated Show resolved Hide resolved

util/argo/audit_logger.go Show resolved Hide resolved

util/argo/audit_logger.go Show resolved Hide resolved

Map string interface

305c1c9

amarrella added 8 commits January 9, 2020 17:01

Sort imports

263de29

Add some tests

aa2b9fd

Structured log

f8c6f9c

Merge branch 'master' into add-audit-logs

0d4a56e

add functional test for failure

8621536

better test

ae201d8

Add success test

c961377

Merge branch 'master' into add-audit-logs

3c1fbb2

amarrella force-pushed the add-audit-logs branch from a89409b to 3c1fbb2 Compare January 13, 2020 16:16

Renamed variables

99fa210

amarrella added 5 commits January 14, 2020 11:36

Add events permission

3bf280a

update also kustomize files

c70b638

Merge branch 'master' into add-audit-logs

30979f0

Add test for PVC creation failure

a85a1c3

Merge branch 'master' into add-audit-logs

ad6c60e

alexec requested changes Jan 21, 2020

View reviewed changes

amarrella added 5 commits January 21, 2020 17:04

allow only create

5db8d72

Add comments on testes and reduce to 30s

7f269aa

Add comments on unit tests

8bd3ea1

Merge branch 'master' into add-audit-logs

de38d56

Merge branch 'master' into add-audit-logs

bc5b662

Increase time to 60s

ac4d732

alexec requested changes Jan 22, 2020

View reviewed changes

alexec approved these changes Jan 22, 2020

View reviewed changes

alexec merged commit 5c98a14 into argoproj:master Jan 22, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(controller): Add audit logs to workflows. Fixes #1769 #1930

feat(controller): Add audit logs to workflows. Fixes #1769 #1930

amarrella commented Jan 9, 2020 •

edited

Loading

codecov bot commented Jan 9, 2020 •

edited

Loading

alexec left a comment

alexec Jan 21, 2020

alexec Jan 21, 2020

alexec Jan 21, 2020

alexec left a comment

amarrella commented Jan 22, 2020 •

edited

Loading

alexec commented Jan 22, 2020

amarrella commented Jan 22, 2020

feat(controller): Add audit logs to workflows. Fixes #1769 #1930

feat(controller): Add audit logs to workflows. Fixes #1769 #1930

Conversation

amarrella commented Jan 9, 2020 • edited Loading

codecov bot commented Jan 9, 2020 • edited Loading

Codecov Report

alexec left a comment

Choose a reason for hiding this comment

alexec Jan 21, 2020

Choose a reason for hiding this comment

alexec Jan 21, 2020

Choose a reason for hiding this comment

alexec Jan 21, 2020

Choose a reason for hiding this comment

alexec left a comment

Choose a reason for hiding this comment

amarrella commented Jan 22, 2020 • edited Loading

alexec commented Jan 22, 2020

amarrella commented Jan 22, 2020

amarrella commented Jan 9, 2020 •

edited

Loading

codecov bot commented Jan 9, 2020 •

edited

Loading

amarrella commented Jan 22, 2020 •

edited

Loading