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

Use the public transport CA as remote CA if the remote CA list is empty #3993

Merged
merged 5 commits into from Dec 3, 2020
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
12 changes: 6 additions & 6 deletions pkg/controller/elasticsearch/certificates/reconcile.go
Expand Up @@ -45,12 +45,7 @@ func Reconcile(
span, _ := apm.StartSpan(ctx, "reconcile_certs", tracing.SpanTypeApp)
defer span.End()

results := &reconciler.Results{}

// reconcile remote clusters certificate authorities
if err := remoteca.Reconcile(driver.K8sClient(), es); err != nil {
results.WithError(err)
}
var results *reconciler.Results

// label certificates secrets with the cluster name
certsLabels := label.NewLabels(k8s.ExtractNamespacedName(&es))
Expand Down Expand Up @@ -113,6 +108,11 @@ func Reconcile(
certRotation,
)

// reconcile remote clusters certificate authorities
if err := remoteca.Reconcile(driver.K8sClient(), es, *transportCA); err != nil {
results.WithError(err)
}

if results.WithResults(transportResults).HasError() {
return nil, results
}
Expand Down
23 changes: 15 additions & 8 deletions pkg/controller/elasticsearch/certificates/remoteca/reconcile.go
Expand Up @@ -36,6 +36,7 @@ func Labels(esName string) client.MatchingLabels {
func Reconcile(
c k8s.Client,
es esv1.Elasticsearch,
ca certificates.CA,
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: transportCA would make things even more explicit

) error {
// Get all the remote certificate authorities
var remoteCAList v1.SecretList
Expand All @@ -46,15 +47,21 @@ func Reconcile(
); err != nil {
return err
}
// We sort the remote certificate authorities to have a stable comparison with the reconciled data
sort.SliceStable(remoteCAList.Items, func(i, j int) bool {
// We don't need to compare the namespace because they are all in the same one
return remoteCAList.Items[i].Name < remoteCAList.Items[j].Name
})

remoteCertificateAuthorities := make([][]byte, len(remoteCAList.Items))
for i, remoteCA := range remoteCAList.Items {
remoteCertificateAuthorities[i] = remoteCA.Data[certificates.CAFileName]
var remoteCertificateAuthorities [][]byte
if len(remoteCAList.Items) > 0 {
// We sort the remote certificate authorities to have a stable comparison with the reconciled data
sort.SliceStable(remoteCAList.Items, func(i, j int) bool {
// We don't need to compare the namespace because they are all in the same one
return remoteCAList.Items[i].Name < remoteCAList.Items[j].Name
})
remoteCertificateAuthorities = make([][]byte, len(remoteCAList.Items))
for i, remoteCA := range remoteCAList.Items {
remoteCertificateAuthorities[i] = remoteCA.Data[certificates.CAFileName]
}
} else {
// if remoteCAList is empty we use the provided transport CA so that we don't end up having an empty cert file mounted on the ES container
remoteCertificateAuthorities = [][]byte{certificates.EncodePEMCert(ca.Cert.Raw)}
}

expected := v1.Secret{
Expand Down
Expand Up @@ -24,7 +24,9 @@ func TestReconcile(t *testing.T) {
type args struct {
es esv1.Elasticsearch
secrets []runtime.Object
ca *certificates.CA
Copy link
Contributor

Choose a reason for hiding this comment

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

shouldn't be a pointer

}
testCA, _ := certificates.NewSelfSignedCA(certificates.CABuilderOptions{})
tests := []struct {
name string
args args
Expand Down Expand Up @@ -59,6 +61,7 @@ func TestReconcile(t *testing.T) {
Data: map[string][]byte{certificates.CAFileName: []byte("cert2\n")},
},
},
ca: testCA,
},
want: []byte("cert2\ncert1\n"),
},
Expand Down Expand Up @@ -101,14 +104,24 @@ func TestReconcile(t *testing.T) {
Data: map[string][]byte{certificates.CAFileName: []byte("cert2\n")},
},
},
ca: testCA,
},
want: []byte("cert2\ncert1\n"),
},
{
name: "Use provided transport CA if remote CA list is empty",
args: args{
es: esv1.Elasticsearch{ObjectMeta: metav1.ObjectMeta{Name: "es1", Namespace: "ns1"}},
ca: testCA,
},
want: certificates.EncodePEMCert(testCA.Cert.Raw),
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
k8sClient := k8s.WrappedFakeClient(tt.args.secrets...)
if err := Reconcile(k8sClient, tt.args.es); (err != nil) != tt.wantErr {
if err := Reconcile(k8sClient, tt.args.es, *tt.args.ca); (err != nil) != tt.wantErr {
t.Errorf("Reconcile() error = %v, wantErr %v", err, tt.wantErr)
}
remoteCaList := v1.Secret{}
Expand Down