Skip to content
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

Secure headers for HTTP Trigger #1229

Merged
merged 2 commits into from May 27, 2021
Merged

Secure headers for HTTP Trigger #1229

merged 2 commits into from May 27, 2021

Conversation

maganaluis
Copy link
Contributor

@maganaluis maganaluis commented May 25, 2021

Checklist:

This solves issue #1227 and it adds support for adding headers to the HTTP Trigger as secrets, so tokens are not committed as code. See the example below:

apiVersion: argoproj.io/v1alpha1
kind: Sensor
metadata:
  name: http-trigger
spec:
  dependencies:
    - name: test-dep
      eventSourceName: webhook
      eventName: example
  triggers:
    - template:
        name: http-trigger
        http:
          secureHeaders:
            - name: serviceToken
              valueFrom:
                secretKeyRef:
                  name: dev-services-secret
                  key: serviceToken
          url: http://http-server.default.svc.cluster.local:8080/hello
          method: GET
      retryStrategy:
        steps: 3
        duration: 3s
      policy:
        status:
          allow:
            - 200
            - 201

@maganaluis maganaluis changed the title Secure headers Secure headers for HTTP Trigger May 25, 2021
Signed-off-by: Luis <[email protected]>

modified scope of http applyresource testcase

Signed-off-by: Luis <[email protected]>

corrected http-trigger documentation

Signed-off-by: Luis <[email protected]>

added pwclabs as a user

Signed-off-by: Luis <[email protected]>
USERS.md Outdated
@@ -19,3 +19,4 @@ Organizations below are **officially** using Argo Events. Please send a PR with
1. [RTL Nederland](https://www.rtl.nl)
1. [Viaduct](https://www.viaduct.ai/)
1. [Woolworths Group](https://www.woolworthsgroup.com.au/)
1. [PwC Labs](https://www.pwc.com/us/en/careers/why-pwc/what-we-do/what-we-do-pwc-labs.html)
Copy link
Member

Choose a reason for hiding this comment

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

ABC order please.

- name: NAMESPACE
valueFrom:
fieldRef:
fieldPath: metadata.namespace
- name: EVENTSOURCE_IMAGE
Copy link
Member

Choose a reason for hiding this comment

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

The order change was cause by different kustomize version. Can you upgrade kustomize to the latest and rerun make manifests? Let me know if you are already running the latest.

@@ -108,6 +108,11 @@ type BasicAuth struct {
Password *corev1.SecretKeySelector `json:"password,omitempty" protobuf:"bytes,2,opt,name=password"`
}

type SecureHeaders struct {
Copy link
Member

Choose a reason for hiding this comment

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

SecureHeader sounds better.

@@ -108,6 +108,11 @@ type BasicAuth struct {
Password *corev1.SecretKeySelector `json:"password,omitempty" protobuf:"bytes,2,opt,name=password"`
}

type SecureHeaders struct {
HeaderName string `json:"headerName,omitempty" protobuf:"bytes,1,opt,name=headerName"`
Copy link
Member

Choose a reason for hiding this comment

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

Maybe just Name?

@@ -108,6 +108,11 @@ type BasicAuth struct {
Password *corev1.SecretKeySelector `json:"password,omitempty" protobuf:"bytes,2,opt,name=password"`
}

type SecureHeaders struct {
HeaderName string `json:"headerName,omitempty" protobuf:"bytes,1,opt,name=headerName"`
Value *corev1.SecretKeySelector `json:"value,omitempty" protobuf:"bytes,2,opt,name=value"`
Copy link
Member

Choose a reason for hiding this comment

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

How about

ValueFrom     *ValueFromSource

type ValueFromSource struct {
  SecretRef *corev1.SecretKeySelector
  ConfigMapRef *corev1.ConfigMapKeySelector
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Are you saying that header names should be read from configmaps and values from secrets? What would be the benefit from reading from configmaps? @whynowy

Copy link
Member

Choose a reason for hiding this comment

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

No. I mean the value could come from configmaps OR secrets - similar to envFrom.

I think this struct could also be reused by the metadata carried by each of the event source, this is another topic.

@maganaluis
Copy link
Contributor Author

@whynowy I tested with both secrets and configmaps. Given you switch to configMapKeyRef.

e.g.

apiVersion: v1
kind: ConfigMap
metadata:
  name: dev-services-secret
  namespace: argo-events
data:
  serviceToken: secret

Copy link
Member

@whynowy whynowy left a comment

Choose a reason for hiding this comment

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

I give my conditional approval, please return make codegen, also an example would be appreciated. Thanks for your contribution!

@maganaluis
Copy link
Contributor Author

@whynowy I re-ran it with the latest Kustomize version and looks like that worked. It still generated event-bus.md and eventsource.md, though I discarded those changes. Let me know if this is good, I don't have access to re-run test cases.

@whynowy
Copy link
Member

whynowy commented May 27, 2021

@whynowy I re-ran it with the latest Kustomize version and looks like that worked. It still generated event-bus.md and eventsource.md, though I discarded those changes. Let me know if this is good, I don't have access to re-run test cases.

No worries, I'll handle that.

@whynowy whynowy merged commit 7a489d1 into argoproj:master May 27, 2021
@Hunter-Thompson
Copy link

@whynowy, any ETA on the next release? Would love this feature!

@whynowy
Copy link
Member

whynowy commented Jun 30, 2021

@whynowy, any ETA on the next release? Would love this feature!

I would expect the BitBucket implementation PR being merged before cutting v1.4, if that doesn't happen, I will tag the version anyways by the end of next week.

juliev0 pushed a commit to juliev0/argo-events that referenced this pull request Mar 29, 2022
* feat: Added secureHeaders to http-trigger

Signed-off-by: Luis <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants