*: implement and test the case where CA exists but App secret doesn't #446
Conversation
|
fanminshi
force-pushed
the
impl_tls_ca_not_app
branch
2 times, most recently
from
a49da2c to
93ef3e2
Compare
fanminshi
changed the title
fanminshi
force-pushed
the
impl_tls_ca_not_app
branch
4 times, most recently
from
589327c to
1e6450a
Compare
| @@ -0,0 +1,18 @@ | |||
| #!/bin/bash | |||
There was a problem hiding this comment.
Sorry, I don't understand where this is used? Is it to generate the certs above after they expire? I think the same thing for the gencert.json, ca.csr, and ca-csr.json above.
Also, should we consider using testdata for the directory rather than fixtures?
Directory and file names that begin with "." or "_" are ignored by the go tool, as are directories named "testdata".
There was a problem hiding this comment.
@shawn-hurley it is used to generate CA key and Cert which is then used by my tests.
func init() {
caCertBytes, err := ioutil.ReadFile("./fixtures/ca.crt")
if err != nil {
panic(err)
}
caConfigMap.Data = map[string]string{tlsutil.TLSCACertKey: string(caCertBytes)}
caKeyBytes, err := ioutil.ReadFile("./fixtures/ca.key")
if err != nil {
panic(err)
}
caSecret.Data = map[string][]byte{tlsutil.TLSPrivateCAKeyKey: caKeyBytes}
}should we consider using testdata for the directory rather than fixtures?
sounds good.
There was a problem hiding this comment.
fixed! PTAL cc/ @shawn-hurley
fanminshi
force-pushed
the
impl_tls_ca_not_app
branch
2 times, most recently
from
311e304 to
d9878fb
Compare
test/e2e/testdata/ca-csr.json
Outdated
| "ca": { | ||
| "expiry": "87600h" | ||
| } | ||
| } No newline at end of file |
fanminshi
force-pushed
the
impl_tls_ca_not_app
branch
from
d9878fb to
a396ab1
Compare
test/e2e/testdata/gencert.json
Outdated
| "expiry": "87600h" | ||
| } | ||
| } | ||
| } No newline at end of file |
fanminshi
force-pushed
the
impl_tls_ca_not_app
branch
from
a396ab1 to
bab166a
Compare
test/e2e/tls_util_test.go
Outdated
| } | ||
| // check if appSecret returned from GenerateCert is the same as the one that exists in the k8s. | ||
| if !reflect.DeepEqual(appSecret, appSecretFromCluster) { | ||
| t.Fatalf("%+v differs %+v", appSecret, appSecretFromCluster) |
There was a problem hiding this comment.
Change this error message to Expected: %+v \n Got: %+v. Otherwise it's not clear what was expected.
There was a problem hiding this comment.
I am going to use "expect %+v, but got %+v" to be consistent.
fanminshi
force-pushed
the
impl_tls_ca_not_app
branch
from
bab166a to
a7bc2cd
Compare
| Name: crName, | ||
| Namespace: namespace, | ||
| }, | ||
| } |
There was a problem hiding this comment.
You might want to make a helper for this, now or in a later PR, since we're using the dummy runtime object in all test cases.
There was a problem hiding this comment.
yeah, I will do another round of refactoring after finishing all the test cases.
fanminshi
force-pushed
the
impl_tls_ca_not_app
branch
from
6f93774 to
fb40804
Compare
|
LGTM |
3 participants
Implement and test the case where CA exists but Application secret doesn't. The CA generator should simply retrieve the CA asset from the cluster and uses it to create a new application TLS secret.