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

Adding support to provide tls version and tls cipher suites. #778

Merged
merged 2 commits into from
May 28, 2024

Conversation

sshver
Copy link
Contributor

@sshver sshver commented May 27, 2024

Issue #, if available:

Description of changes:

  • Adding options to provide tls version and tls cipher suites.

Additional testing:

  • Case where a wrong parameters are provided.
helm upgrade appmesh-controller ./config/helm/appmesh-controller --namespace appmesh-system \
--set region=us-west-2 \
--set serviceAccount.create=false \
--set image.repository=407406722945.dkr.ecr.us-west-2.amazonaws.com/test-tls-version \
--set image.tag=test \
--set clusterName=test-app-mesh \
--set tlsMinVersion=VersionTLS13 \
--set tlsCipherSuite="TLS_256_CBC_SHA\,TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384"
aws-app-mesh-controller-for-k8s git:(master) ✗ kubectl logs appmesh-controller-75888db8df-4z6kd -n appmesh-system
{"level":"info","ts":"2024-05-27T06:50:24Z","logger":"setup","msg":"version","GitVersion":"v1.12.7-10-g48e1f73-dirty","GitCommit":"48e1f738c9dd697728eeeaa7b8728d98b0027cbd","BuildDate":"2024-05-26T23:25:57+0000"}
{"level":"info","ts":"2024-05-27T06:50:24Z","logger":"setup","msg":"Health endpoint","HealthProbeBindAddress":":61779"}
{"level":"info","ts":"2024-05-27T06:50:24Z","logger":"setup","msg":"TlsVersion","TLSVersion":"VersionTLS13"}
{"level":"info","ts":"2024-05-27T06:50:24Z","logger":"setup","msg":"TlsCipherSuite","TlsCipherSuite":["TLS_256_CBC_SHA","TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384"]}
{"level":"info","ts":"2024-05-27T06:50:24Z","logger":"controller-runtime.metrics","msg":"Metrics server is starting to listen","addr":"0.0.0.0:8080"}
{"level":"info","ts":"2024-05-27T06:50:24Z","logger":"setup","msg":"Cluster Name","ClusterName":"test-app-mesh"}
{"level":"info","ts":"2024-05-27T06:50:25Z","logger":"setup","msg":"IpFamily of the cluster","IpFamily":"ipv4"}
. . . .
. . . . 
{"level":"info","ts":"2024-05-27T06:50:25Z","logger":"setup","msg":"starting controller"}
{"level":"info","ts":"2024-05-27T06:50:25Z","logger":"controller-runtime.webhook.webhooks","msg":"Starting webhook server"}
{"level":"info","ts":"2024-05-27T06:50:25Z","logger":"controller-runtime.certwatcher","msg":"Updated current TLS certificate"}
{"level":"error","ts":"2024-05-27T06:50:25Z","logger":"setup","msg":"Failed to convert TLS cipher suite name to ID","error":"Cipher suite TLS_256_CBC_SHA not supported or doesn't exist","stacktrace":"main.main.func1\n\t/workspace/main.go:176\nsigs.k8s.io/controller-runtime/pkg/webhook.(*Server).Start\n\t/go/pkg/mod/sigs.k8s.io/[email protected]/pkg/webhook/server.go:263\nsigs.k8s.io/controller-runtime/pkg/manager.(*runnableGroup).reconcile.func1\n\t/go/pkg/mod/sigs.k8s.io/[email protected]/pkg/manager/runnable_group.go:219"}
. . . .
. . . .
{"level":"error","ts":"2024-05-26T23:45:12Z","logger":"setup","msg":"TLS version invalid","error":"unknown tls version \"VersionTLS123\"","stacktrace":"main.main.func1\n\t/workspace/main.go:168\nsigs.k8s.io/controller-runtime/pkg/webhook.(*Server).Start\n\t/go/pkg/mod/sigs.k8s.io/[email protected]/pkg/webhook/server.go:263\nsigs.k8s.io/controller-runtime/pkg/manager.(*runnableGroup).reconcile.func1\n\t/go/pkg/mod/sigs.k8s.io/[email protected]/pkg/manager/runnable_group.go:219"}
  • Default case:
helm upgrade appmesh-controller ./config/helm/appmesh-controller --namespace appmesh-system \
--set region=us-west-2 \
--set serviceAccount.create=false \
--set image.repository=407406722945.dkr.ecr.us-west-2.amazonaws.com/test-tls-version \
--set image.tag=test \
--set clusterName=test-app-mesh  
aws-app-mesh-controller-for-k8s git:(master) ✗ kubectl logs appmesh-controller-7d9c7b5949-gfm7r  -n appmesh-system
{"level":"info","ts":"2024-05-27T00:51:37Z","logger":"setup","msg":"version","GitVersion":"v1.12.7-10-g48e1f73-dirty","GitCommit":"48e1f738c9dd697728eeeaa7b8728d98b0027cbd","BuildDate":"2024-05-26T23:25:57+0000"}
{"level":"info","ts":"2024-05-27T00:51:37Z","logger":"setup","msg":"Health endpoint","HealthProbeBindAddress":":61779"}
{"level":"info","ts":"2024-05-27T00:51:37Z","logger":"setup","msg":"TlsVersion","TLSVersion":"VersionTLS12"}
{"level":"info","ts":"2024-05-27T00:51:37Z","logger":"setup","msg":"TlsCipherSuite","TlsCipherSuite":[]}
. . . .
. . . .
{"level":"info","ts":"2024-05-27T00:51:37Z","logger":"setup","msg":"starting controller"}
{"level":"info","ts":"2024-05-27T00:51:37Z","msg":"Starting server","kind":"health probe","addr":"[::]:61779"}
{"level":"info","ts":"2024-05-27T00:51:37Z","logger":"controller-runtime.webhook.webhooks","msg":"Starting webhook 
. . . . 
. . . .
  • Case with correct tls version and tls cipher suite is passed
helm install appmesh-controller ./config/helm/appmesh-controller --namespace appmesh-system \
--set region=us-west-2 \                     
--set serviceAccount.create=false \                                  
--set image.repository=407406722945.dkr.ecr.us-west-2.amazonaws.com/test-tls-version \--set image.tag=test \--set clusterName=test-app-mesh \
--set tlsMinVersion=VersionTLS13 \
--set tlsCipherSuite="TLS_RSA_WITH_AES_256_CBC_SHA\,TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384"
aws-app-mesh-controller-for-k8s git:(master) ✗ kubectl logs appmesh-controller-65f69bc88c-6dq5g -n appmesh-system 
{"level":"info","ts":"2024-05-27T06:38:18Z","logger":"setup","msg":"version","GitVersion":"v1.12.7-10-g48e1f73-dirty","GitCommit":"48e1f738c9dd697728eeeaa7b8728d98b0027cbd","BuildDate":"2024-05-26T23:25:57+0000"}
{"level":"info","ts":"2024-05-27T06:38:18Z","logger":"setup","msg":"Health endpoint","HealthProbeBindAddress":":61779"}
{"level":"info","ts":"2024-05-27T06:38:18Z","logger":"setup","msg":"TlsVersion","TLSVersion":"VersionTLS13"}
{"level":"info","ts":"2024-05-27T06:38:18Z","logger":"setup","msg":"TlsCipherSuite","TlsCipherSuite":["TLS_RSA_WITH_AES_256_CBC_SHA","TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384"]}
{"level":"info","ts":"2024-05-27T06:38:18Z","logger":"controller-runtime.metrics","msg":"Metrics server is starting to listen","addr":"0.0.0.0:8080"}
{"level":"info","ts":"2024-05-27T06:38:19Z","logger":"setup","msg":"Cluster Name","ClusterName":"test-app-mesh"}
{"level":"info","ts":"2024-05-27T06:38:19Z","logger":"setup","msg":"IpFamily of the cluster","IpFamily":"ipv4"}
. . . . 
. . . .
{"level":"info","ts":"2024-05-27T06:38:19Z","logger":"setup","msg":"starting controller"}
{"level":"info","ts":"2024-05-27T06:38:19Z","logger":"controller-runtime.webhook.webhooks","msg":"Starting webhook server"}
{"level":"info","ts":"2024-05-27T06:38:19Z","msg":"Starting server","kind":"health probe","addr":"[::]:61779"}
{"level":"info","ts":"2024-05-27T06:38:19Z","msg":"Starting server","path":"/metrics","kind":"metrics","addr":"[::]:8080"}
{"level":"info","ts":"2024-05-27T06:38:19Z","logger":"controller-runtime.certwatcher","msg":"Updated current TLS certificate"}
{"level":"info","ts":"2024-05-27T06:38:19Z","logger":"controller-runtime.webhook","msg":"Serving webhook server","host":"","port":9443}
. . . .
. . . .
{"level":"info","ts":"2024-05-27T06:38:19Z","msg":"Starting Controller","controller":"virtualrouter","controllerGroup":"appmesh.k8s.aws","controllerKind":"VirtualRouter"}
{"level":"info","ts":"2024-05-27T06:38:19Z","logger":"setup","msg":"starting custom controller"}
. . . .
. . . .

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

Copy link
Contributor

@joesbigidea joesbigidea left a comment

Choose a reason for hiding this comment

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

Looks good

@joesbigidea joesbigidea merged commit 0b92784 into aws:master May 28, 2024
3 checks passed
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.

2 participants