cmd/contour: adds sds cli #1048
Conversation
|
The review and commit order would be
|
mattalberts
force-pushed
the
xds-sds-cli
branch
2 times, most recently
from
48c93c1
to
87d29a0
Compare
mattalberts
force-pushed
the
xds-sds-cli
branch
2 times, most recently
from
dffcf6e
to
93d4a1a
Compare
internal/dag/dag.go
Outdated
| @@ -293,6 +293,16 @@ func (s *Secret) Data() map[string][]byte { | |||
| return s.Object.Data | |||
| } | |||
|
|
|||
| // Cert returns the secret's tls certificate | |||
| func (s *Secret) Cert() []byte { | |||
| return s.Object.Data[v1.TLSCertKey] | |||
There was a problem hiding this comment.
Could there be a case where the cert is not in the secret?
There was a problem hiding this comment.
It's plausible. It is assumed to be valued when setting up the downstream context; but on review, that isn't guaranteed. If the map key were missing, the vhost would be given an empty array of bytes right now.
envoy.DownstreamTLSContext(data[v1.TLSCertKey], data[v1.TLSPrivateKeyKey], vh.MinProtoVersion, alpnProtos...)- https://github.com/heptio/contour/blob/master/internal/contour/listener.go#L265
It feels like the the structure should be verified while creating a SecureVirtualHost vertex.
What about as part of func (b *builder) lookupSecret(m meta) *Secret? There is handling for when the function returns nil; missing structure could value the return as nil (there just needs to be a way to signal that to the user via an event or log message).
There was a problem hiding this comment.
These got marked as outdated when I rebased. They were brought in with #1047.
There was a problem hiding this comment.
ok no biggie, there's another issue where we get the cert and not the key and it causes issues, but let's not try and fix that in this (just been on my mind).
There was a problem hiding this comment.
Its okay! I'm glad you mentioned it. I remember looking at it while I drafted the PR, when deciding to introduce the helper functions on *dag.Secret. Is there already an issue for that? I can clean them all up at one time in another PR :).
There was a problem hiding this comment.
I bet its this one, #1051 :)
internal/dag/dag.go
Outdated
|
|
||
| // PrivateKey returns the secret's tls private key | ||
| func (s *Secret) PrivateKey() []byte { | ||
| return s.Object.Data[v1.TLSPrivateKeyKey] |
There was a problem hiding this comment.
Same here if the key is nil?
There was a problem hiding this comment.
Agreed. If the key doesn't exist, the return would be populated as and empty []byte.
Addresses the third high-level design bullet point, support sds in the contour cli for debugging. Updates: projectcontour#898 Signed-off-by: Matt Alberts <malberts@cloudflare.com>
mattalberts
force-pushed
the
xds-sds-cli
branch
from
93d4a1a
to
64632ca
Compare
Addresses the third high-level design bullet point, support sds in the
contour cli for debugging.
Updates: #898
Signed-off-by: Matt Alberts malberts@cloudflare.com