-
Notifications
You must be signed in to change notification settings - Fork 875
feat: Adding pubsub interface #2538
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
Changes from 18 commits
acec1fe
c91fcb9
2d795e8
4da87c6
ce86992
fb3a791
21f3ccd
cced7f7
4a557d5
2581824
2ac3adc
20010f0
e8793e4
e9a1559
f8a60f6
f3ddfb9
76af8d0
9be106a
561839a
17e0271
2cf8b7c
6bf81ca
43acf63
a09ff0e
1319b9e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,69 @@ | ||
| name: dapr-pubsub | ||
| on: | ||
| push: | ||
| paths: | ||
| - "pkg/pubsub/dapr" | ||
|
JaydipGabani marked this conversation as resolved.
|
||
| - "test/pubsub/**" | ||
| pull_request: | ||
| paths: | ||
| - "pkg/pubsub/dapr" | ||
| - "test/pubsub/**" | ||
| permissions: read-all | ||
|
|
||
|
JaydipGabani marked this conversation as resolved.
|
||
| jobs: | ||
| dapr_test: | ||
| name: "Dapr pubsub test" | ||
| runs-on: ubuntu-22.04 | ||
| timeout-minutes: 15 | ||
| strategy: | ||
| matrix: | ||
| DAPR_VERSION: ["1.10"] | ||
| steps: | ||
| - name: Check out code into the Go module directory | ||
| uses: actions/checkout@ac593985615ec2ede58e132d2e21d2b1cbd6127c | ||
|
|
||
| - name: Set up Go 1.20 | ||
| uses: actions/setup-go@v3 | ||
| with: | ||
| go-version: 1.20 | ||
|
|
||
| - name: Bootstrap e2e | ||
| run: | | ||
| mkdir -p $GITHUB_WORKSPACE/bin | ||
| mkdir .tmp | ||
| echo "$GITHUB_WORKSPACE/bin" >> $GITHUB_PATH | ||
| make e2e-bootstrap | ||
| make e2e-helm-install | ||
| helm repo add dapr https://dapr.github.io/helm-charts/ | ||
| helm repo add bitnami https://charts.bitnami.com/bitnami | ||
|
JaydipGabani marked this conversation as resolved.
|
||
| helm repo update | ||
| helm upgrade --install dapr dapr/dapr --version=${{ matrix.DAPR_VERSION }} --namespace dapr-system --create-namespace --wait --debug | ||
| helm upgrade --install redis bitnami/redis --namespace default --set image.tag=7.0-debian-11 --wait --debug | ||
| make e2e-subscriber-build-load-image | ||
| make e2e-subscriber-deploy | ||
|
|
||
| - name: Run e2e | ||
| run: | | ||
| make docker-buildx IMG=gatekeeper-e2e:latest | ||
| make e2e-build-load-externaldata-image | ||
| make docker-buildx-crds CRD_IMG=gatekeeper-crds:latest | ||
| kind load docker-image --name kind gatekeeper-e2e:latest gatekeeper-crds:latest | ||
| kubectl create ns gatekeeper-system | ||
|
JaydipGabani marked this conversation as resolved.
|
||
| make e2e-publisher-deploy | ||
| make e2e-helm-deploy HELM_REPO=gatekeeper-e2e HELM_CRD_REPO=gatekeeper-crds HELM_RELEASE=latest ENABLE_PUBSUB=true | ||
| make test-e2e ENABLE_PUBSUB_TESTS=1 | ||
|
|
||
| - name: Save logs | ||
| if: ${{ always() }} | ||
| run: | | ||
| kubectl logs -n fake-subscriber -l app=sub --tail=-1 > logs-audit-subscribe.json | ||
| kubectl logs -n gatekeeper-system -l control-plane=audit-controller --tail=-1 > logs-audit-publish.json | ||
|
|
||
| - name: Upload artifacts | ||
| uses: actions/upload-artifact@v3 | ||
| if: ${{ always() }} | ||
| with: | ||
| name: pubsub-logs | ||
| path: | | ||
| logs-*.json | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -362,3 +362,6 @@ tilt-settings.json | |
|
|
||
| # dummy provider's certificate directory | ||
| test/externaldata/dummy-provider/certs/ | ||
|
|
||
| # annotations for dapr | ||
| annotations.yaml | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. i think we added this to .tmp, do we still need this?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I reverted that change back, since creating
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why is the directory nonexistent? Is there a way to remedy this? It would be good to avoid clutter in the root directory: saying "
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. bump (was not addressed since last review request)
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. My bad. I addressed it. Creating a tmp dir while bootstrapping to mitigate
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Awesome, thanks! |
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -75,6 +75,10 @@ var replacements = map[string]string{ | |
| {{- toYaml .Values.podAnnotations | trim | nindent 8 }} | ||
| {{- end }}`, | ||
|
|
||
| `HELMSUBST_PUBSUB_ANNOTATIONS: ""`: `{{- if .Values.podPubsubAnnotations }} | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why do we need a special knob for pubsub annotations when users can already fully customize annotations? What are these annotations used for?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For now, we dont want to inject dapr's pubsub annotation to
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For anyother drivers, if they need any specific annotation on the pod running pubsub, this might be a way to provide it only for audit.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We should make this field something like AUDIT_POD_ANNOTATIONS, then. This is more consistent with our stance of not explicitly supporting specific 3rd party tooling in helm, but providing generic knobs that allow for support.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Updated |
||
| {{- toYaml .Values.podPubsubAnnotations | trim | nindent 8 }} | ||
| {{- end }}`, | ||
|
|
||
| "HELMSUBST_SECRET_ANNOTATIONS": `{{- toYaml .Values.secretAnnotations | trim | nindent 4 }}`, | ||
|
|
||
| "- HELMSUBST_TLS_HEALTHCHECK_ENABLED_ARG": `{{ if .Values.enableTLSHealthcheck}}- --enable-tls-healthcheck{{- end }}`, | ||
|
|
@@ -85,6 +89,12 @@ var replacements = map[string]string{ | |
|
|
||
| "- HELMSUBST_MUTATION_STATUS_ENABLED_ARG": `{{ if not .Values.disableMutation}}- --operation=mutation-status{{- end }}`, | ||
|
|
||
| "- HELMSUBST_PUBSUB_ARGS": `{{ if .Values.audit.enablePubsub}} | ||
| - --enable-pub-sub={{ .Values.audit.enablePubsub }} | ||
| - --audit-connection={{ .Values.audit.connection }} | ||
| - --audit-channel={{ .Values.audit.channel }} | ||
| {{- end }}`, | ||
|
|
||
| "HELMSUBST_MUTATING_WEBHOOK_FAILURE_POLICY": `{{ .Values.mutatingWebhookFailurePolicy }}`, | ||
|
|
||
| "HELMSUBST_MUTATING_WEBHOOK_REINVOCATION_POLICY": `{{ .Values.mutatingWebhookReinvocationPolicy }}`, | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.