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

enhancement: Add token based auth as alternative to basic-auth for http requests. #192

Open
wants to merge 14 commits into
base: master
Choose a base branch
from
Open
9 changes: 8 additions & 1 deletion .github/workflows/build_and_test.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ jobs:

wait_for_pod_ready "sidecar"
wait_for_pod_ready "sidecar-5xx"
wait_for_pod_ready "sidecar-api_key"
wait_for_pod_ready "dummy-server-pod"

- name: Install Configmaps and Secrets
Expand All @@ -80,6 +81,7 @@ jobs:
- name: Retrieve pod logs
run: |
kubectl logs sidecar > /tmp/sidecar.log
kubectl logs sidecar > /tmp/sidecar-api_key.log
kubectl logs sidecar-5xx > /tmp/sidecar-5xx.log
kubectl logs dummy-server-pod > /tmp/dummy-server.log
- name: Upload artifacts
Expand Down Expand Up @@ -109,6 +111,9 @@ jobs:
kubectl cp sidecar-5xx:/tmp-5xx/relative/relative.txt /tmp/5xx/relative.txt
kubectl cp sidecar-5xx:/tmp-5xx/500.txt /tmp/5xx/500.txt

echo "Downloading resource files from sidecar-api_key..."
kubectl cp sidecar-api_key:/tmp/api_key.txt /tmp/api-key/api_key.txt || true

- name: Verify files
run: |
echo "Verifying file content from sidecar and sidecar-5xx ..."
Expand All @@ -126,4 +131,6 @@ jobs:
echo -n "This absolutely exists" | diff - /tmp/5xx/absolute.txt &&
echo -n "This relatively exists" | diff - /tmp/5xx/relative.txt &&
echo -n "500" | diff - /tmp/5xx/500.txt &&
ls /tmp/5xx/script_result
ls /tmp/5xx/script_result &&
[ -f /tmp/api-key/api_key.txt ] && echo "api_key used for aouthentication"

2 changes: 2 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
*.iml
.idea/
venv
.vscode/
__pycache__
4 changes: 3 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ metadata:
k8s-sidecar-target-directory: "/path/to/target/directory"
```

If the filename ends with `.url` suffix, the content will be processed as a URL which the target file contents will be downloaded from.
If the filename ends with `.url` suffix, the content will be processed as a URL which the target file contents will be downloaded from. If the source requires authentication, currently http basic-auth and token based authentication via query param are supported. Please see the variables below for details.

## Configuration Environment Variables

Expand All @@ -69,6 +69,8 @@ If the filename ends with `.url` suffix, the content will be processed as a URL
| `REQ_RETRY_READ` | How many times to retry on read errors for any http request (`.url` triggered requests, requests to `REQ_URI` and k8s api requests) | false | `5` | integer |
| `REQ_RETRY_BACKOFF_FACTOR` | A backoff factor to apply between attempts after the second try for any http request (`.url` triggered requests, requests to `REQ_URI` and k8s api requests) | false | `1.1` | float |
| `REQ_TIMEOUT` | How many seconds to wait for the server to send data before giving up for `.url` triggered requests or requests to `REQ_URI` (does not apply to k8s api requests) | false | `10` | float |
| `REQ_TOKEN_KEY` | Key of the key/value pair passed as parameter within the url for token based authentication for requests to `REQ_URL` and for `*.url` triggered requests. If `REQ_TOKEN_VALUE` is set and `REQ_TOKEN_KEY` is not, it defaults to "`private_token`". | false | - | string |
| `REQ_TOKEN_VALUE` | Value of the key/value pair passed as parameter within the url for token based authentication for requests to `REQ_URL` and for `*.url` triggered requests. If configured it takes precedence over eventually also configured basic-auth credentials (`REQ_USERNAME`/`REQ_PASSWORD`) | false | - | string |
| `REQ_USERNAME` | Username to use for basic authentication for requests to `REQ_URL` and for `*.url` triggered requests | false | - | string |
| `REQ_PASSWORD` | Password to use for basic authentication for requests to `REQ_URL` and for `*.url` triggered requests | false | - | string |
| `SCRIPT` | Absolute path to shell script to execute after a configmap got reloaded. It runs before calls to `REQ_URI` | false | - | string |
Expand Down
16 changes: 16 additions & 0 deletions example.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,10 @@ spec:
volumeMounts:
- name: shared-volume
mountPath: /tmp/
# Only relevant for token based auth (like with git)
# envFrom:
# - secretRef:
# name: token-auth-sample-secret
env:
- name: LABEL
value: "findme"
Expand Down Expand Up @@ -71,6 +75,18 @@ data:
# base64 encoded: my super cool \n multiline \ secret
secret.world: bXkgc3VwZXIgY29vbAptdWx0aWxpbmUKc2VjcmV0
---
# This secret is only necessary for token based authentication of http requests.
# Refer to README.md for details on the two parameters REQ_TOKEN_KEY and REQ_TOKEN_VALUE
apiVersion: v1
kind: Secret
metadata:
name: token-auth-sample-secret
type: Opaque
data:
#REQ_TOKEN_KEY: private_token
# base64 encoded super-duper-secret authentication token
REQ_TOKEN_VALUE: c3VwZXItZHVwZXItc2VjcmV0
---
apiVersion: rbac.authorization.k8s.io/v1
kind: ClusterRole
metadata:
Expand Down
22 changes: 19 additions & 3 deletions src/helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -102,10 +102,26 @@ def request(url, method, enable_5xx=False, payload=None):

username = os.getenv("REQ_USERNAME")
password = os.getenv("REQ_PASSWORD")
if username and password:
req_token_key = os.getenv("REQ_TOKEN_KEY")
req_token_value = os.getenv("REQ_TOKEN_VALUE")
params=dict()

if req_token_value and not req_token_key:
req_token_key = "private_token"
logger.info(f"Request token value is set, but key is not. Setting request token key to default value {req_token_key}.")
jekkel marked this conversation as resolved.
Show resolved Hide resolved

if req_token_key and req_token_value:
auth = None
params[ str(req_token_key) ] = req_token_value
logger.debug(f"Token based authorization configured. Token key: {req_token_key}.")
elif username and password:
auth = (username, password)
params = None
logger.debug(f"Basic-auth configured. username: {username}.")
else:
logger.debug(f"No authorization tokens set (no basic-auth and no private token).")
auth = None
params = None

r = requests.Session()

Expand All @@ -123,9 +139,9 @@ def request(url, method, enable_5xx=False, payload=None):

# If method is not provided use GET as default
if method == "GET" or not method:
res = r.get("%s" % url, auth=auth, timeout=REQ_TIMEOUT)
res = r.get("%s" % url, auth=auth, params=params, timeout=REQ_TIMEOUT)
elif method == "POST":
res = r.post("%s" % url, auth=auth, json=payload, timeout=REQ_TIMEOUT)
res = r.post("%s" % url, auth=auth, params=params, json=payload, timeout=REQ_TIMEOUT)
else:
logger.warning(f"Invalid REQ_METHOD: '{method}', please use 'GET' or 'POST'. Doing nothing.")
return
Expand Down
9 changes: 9 additions & 0 deletions test/resources/resources.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -51,3 +51,12 @@ metadata:
findme: "yup"
data:
500.txt.url: "http://dummy-server/500"
---
apiVersion: v1
kind: ConfigMap
metadata:
name: url-configmap-api_key
labels:
findme: "sure"
data:
api_key.txt.url: "http://dummy-server/200/api_key"
45 changes: 45 additions & 0 deletions test/resources/sidecar.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,51 @@ spec:
defaultMode: 0777
---
apiVersion: v1
kind: Secret
metadata:
name: token-auth-sample-secret
type: Opaque
data:
REQ_TOKEN_KEY: private_token
REQ_TOKEN_VALUE: c3VwZXItZHVwZXItc2VjcmV0
---
apiVersion: v1
kind: Pod
metadata:
name: sidecar-api_key
namespace: default
spec:
serviceAccountName: sample-acc
containers:
- name: sidecar
image: kiwigrid/k8s-sidecar:testing
volumeMounts:
- name: shared-volume
mountPath: /tmp/
- name: script-volume
Copy link
Member

Choose a reason for hiding this comment

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

I think we don't need the script here.

Copy link
Author

Choose a reason for hiding this comment

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

good point. removed the script

mountPath: /opt/script.sh
subPath: script.sh
envFrom:
- secretRef:
name: token-auth-sample-secret
env:
- name: LABEL
value: "findme"
- name: FOLDER
value: /tmp/
- name: RESOURCE
value: both
- name: SCRIPT
Copy link
Member

Choose a reason for hiding this comment

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

and here

Copy link
Author

Choose a reason for hiding this comment

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

good point. removed the script

Copy link
Author

Choose a reason for hiding this comment

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

also switched resource to configmap

value: "/opt/script.sh"
volumes:
- name: shared-volume
emptyDir: {}
- name: script-volume
Copy link
Member

Choose a reason for hiding this comment

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

and here

Copy link
Author

Choose a reason for hiding this comment

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

good point. removed the script

configMap:
name: script-configmap
defaultMode: 0777
---
apiVersion: v1
kind: Pod
metadata:
name: dummy-server-pod
Expand Down
21 changes: 19 additions & 2 deletions test/server/server.py
Original file line number Diff line number Diff line change
@@ -1,8 +1,19 @@
from fastapi import FastAPI
import uvicorn
#from http.client import HTTPException, HTTPResponse
from fastapi import FastAPI, Security, Depends, HTTPException
from fastapi.security.api_key import APIKeyQuery, APIKey
#import uvicorn
jekkel marked this conversation as resolved.
Show resolved Hide resolved

API_KEY_NAME="private_token"
API_KEY="super-duper-secret"
api_key_query = APIKeyQuery(name=API_KEY_NAME, auto_error=True)

app = FastAPI()

def get_api_key (api_key_query: str = Security(api_key_query)):
if api_key_query == API_KEY:
return api_key_query
else:
raise HTTPException(403)

@app.get("/", status_code=200)
def read_root():
Expand All @@ -27,3 +38,9 @@ async def read_item():
@app.post("/503", status_code=503)
async def read_item():
return 503


@app.get("/200/api_key")
Copy link
Member

Choose a reason for hiding this comment

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

could you add another endpoint with a different API key so we can add a negative test case as well? That would be awesome!

Copy link
Author

Choose a reason for hiding this comment

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

added a new container with wrong credentials/secret and used the same endpoint

def read_root(api_key: APIKey = Depends(get_api_key)):
return 200