Test authn-k8s on Conjur-OSS deployed via Helm chart on OpenShift #131
Conversation
doodlesbykumbi
force-pushed
the
test-oss
branch
5 times, most recently
from
e2341f8 to
49577ca
Compare
Jenkinsfile
Outdated
| } | ||
| } | ||
|
|
||
| stage('OpenShift v4.5, v5 Conjur OSS, Postgres, Annotation-based Authn') { |
There was a problem hiding this comment.
Should this test be grouped with the other Annotation-based below?
There was a problem hiding this comment.
Same for the Host-ID-based Authn stage above this one?
Or, if these Conjur OSS tests can't be merged below, the comment on line 16 should be changed or moved.
|
@doodlesbykumbi I've been talking with @BradleyBoutcher about adding quick start flows (draft content here) to The end goal of centralizing all of this content is that we can start to provide super simple patterns for running different Conjur editions in test automation, so that new projects have an easy way to get started writing integration tests. ETA: not for this PR, obviously - just wanted to raise this as a future-thinking option. |
Jenkinsfile
Outdated
| } | ||
| } | ||
|
|
||
| stage('OpenShift v4.5, v5 Conjur OSS, Postgres, Annotation-based Authn') { |
There was a problem hiding this comment.
Same for the Host-ID-based Authn stage above this one?
Or, if these Conjur OSS tests can't be merged below, the comment on line 16 should be changed or moved.
ci/test
Outdated
| runDockerCommand " | ||
| ./stop | ||
| helm --namespace '$CONJUR_NAMESPACE_NAME' delete '${CONJUR_OSS_HELM_RELEASE_NAME}' | ||
| kubectl delete namespace $CONJUR_NAMESPACE_NAME |
There was a problem hiding this comment.
Should probably use a "$cli" here instead of kubectl to switch between oc and kubectl.
We can also consider adding a comment timeout since we're deleting a namespace. Namespace deletion can involve deleting many objectss, including some objects like LoadBalancers that use finalizers, and sometimes there are bugs whereby finalizers aren't getting cleared. Granted, this only came into play for OC 4.3 so far, but maybe good to add a timeout?
I don't know if it makes sense to go this deeply, but in the stop script, we're also describing residual objects in the namespace:
"$cli" delete --timeout="$KUBE_CLI_DELETE_TIMEOUT" \
namespace "$TEST_APP_NAMESPACE_NAME" || \
(echo "ERROR: Delete of namespace $TEST_APP_NAMESPACE_NAME failed" && \
echo "Showing residual resources in namespace:" && \
"$cli" describe all -n "$TEST_APP_NAMESPACE_NAME")
Might be enough just to display an error, without the residual objects?
There was a problem hiding this comment.
I think both are good ideas. The residual objects will be great for debugging.
There was a problem hiding this comment.
I thought about the $cli and realised there's no loss in generality by using only kubectl, especially since all the changes in this PR are for testing only but also because
kubectlis available in the test container- Both OpenShift and Kubernetes respect the Kubernetes API
There was a problem hiding this comment.
Yes, I think this makes sense, and would keep things more KISS.
| --set service.external.enabled=false \ | ||
| --set postgres.persistentVolume.create=false \ | ||
| --set rbac.create=true \ | ||
| --set 'dataKey=To7gsAFQOm7NlVnBWA3gFYlZIHC25pGwm/pMxlcHLUY=' \ |
There was a problem hiding this comment.
Peripheral comment: It will be nice to have the Helm chart automatically generate a dataKey (cyberark/conjur-oss-helm-chart#136)
There was a problem hiding this comment.
It would! I also realised that we tend to generate certain things (like the datakey) that can be static for the sake of testing.
|
|
||
| cd '${workdir}' | ||
|
|
||
| kubectl create namespace '${CONJUR_NAMESPACE_NAME}' |
There was a problem hiding this comment.
kubectl here should be oc? Same for other instances of kubectl below.
rpothier
force-pushed
the
test-oss
branch
5 times, most recently
from
2f1da19 to
5cbdd00
Compare
Namespace deletion can fail or hang indefinitely for a variety of reasons. A timeout helps to prevent against this.
|
LGTM |
What does this PR do ?
TODO: it's probably better to move this functionality to kubernetes-conjur-deploy.
TODO: there really needs to be a better way of passing along conjur config to kubernetes-conjur-demo (fortunately we're working on this). At present kubernetes-conjur-demo seems too tightly coupled with kubernetes-conjur-deploy and knows too much about it, this makes it more difficult to make updates to either.
Resolves #110