Skip to content

Commit 3d0fba3

Browse files
shuheiktgwshuheiktgw
committed
fix: Add nil check for Extensions in GetLivenessProbe and GetReadinessProbe functions
1 parent d7291dd commit 3d0fba3

File tree

2 files changed

+347
-0
lines changed

2 files changed

+347
-0
lines changed

‎apis/v1beta1/config.go

+8
Original file line numberDiff line numberDiff line change
@@ -343,6 +343,10 @@ func (c *Config) ApplyDefaults(logger logr.Logger) error {
343343
// GetLivenessProbe gets the first enabled liveness probe. There should only ever be one extension enabled
344344
// that provides the hinting for the liveness probe.
345345
func (c *Config) GetLivenessProbe(logger logr.Logger) (*corev1.Probe, error) {
346+
if c.Extensions == nil {
347+
return nil, nil
348+
}
349+
346350
enabledComponents := c.GetEnabledComponents()
347351
for componentName := range enabledComponents[KindExtension] {
348352
// TODO: Clean up the naming here and make it simpler to use a retriever.
@@ -359,6 +363,10 @@ func (c *Config) GetLivenessProbe(logger logr.Logger) (*corev1.Probe, error) {
359363
// GetReadinessProbe gets the first enabled readiness probe. There should only ever be one extension enabled
360364
// that provides the hinting for the readiness probe.
361365
func (c *Config) GetReadinessProbe(logger logr.Logger) (*corev1.Probe, error) {
366+
if c.Extensions == nil {
367+
return nil, nil
368+
}
369+
362370
enabledComponents := c.GetEnabledComponents()
363371
for componentName := range enabledComponents[KindExtension] {
364372
// TODO: Clean up the naming here and make it simpler to use a retriever.

‎apis/v1beta1/config_test.go

+339
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import (
1111
"testing"
1212

1313
"github.com/go-logr/logr"
14+
"github.com/google/go-cmp/cmp"
1415
"github.com/stretchr/testify/assert"
1516
"github.com/stretchr/testify/require"
1617
go_yaml "gopkg.in/yaml.v3"
@@ -758,3 +759,341 @@ func TestConfig_GetExporterPorts(t *testing.T) {
758759
})
759760
}
760761
}
762+
763+
func TestConfig_GetLivenessProbe(t *testing.T) {
764+
tests := []struct {
765+
name string
766+
config *Config
767+
wantProbe *v1.Probe
768+
wantErr bool
769+
}{
770+
{
771+
name: "nil extensions should return nil",
772+
config: &Config{
773+
Extensions: nil,
774+
Service: Service{
775+
Extensions: []string{},
776+
},
777+
},
778+
wantProbe: nil,
779+
},
780+
{
781+
name: "nil extensions with health_check in service extensions should return nil",
782+
config: &Config{
783+
Extensions: nil,
784+
Service: Service{
785+
Extensions: []string{"health_check"},
786+
},
787+
},
788+
wantProbe: nil,
789+
},
790+
{
791+
name: "empty extensions should return nil",
792+
config: &Config{
793+
Extensions: &AnyConfig{
794+
Object: map[string]interface{}{},
795+
},
796+
Service: Service{
797+
Extensions: []string{},
798+
},
799+
},
800+
wantProbe: nil,
801+
},
802+
{
803+
name: "empty extensions with health_check in service extensions should return probe",
804+
config: &Config{
805+
Extensions: &AnyConfig{
806+
Object: map[string]interface{}{},
807+
},
808+
Service: Service{
809+
Extensions: []string{"health_check"},
810+
},
811+
},
812+
wantProbe: &v1.Probe{
813+
ProbeHandler: v1.ProbeHandler{
814+
HTTPGet: &v1.HTTPGetAction{
815+
Path: "/",
816+
Port: intstr.FromInt32(13133),
817+
},
818+
},
819+
},
820+
},
821+
{
822+
name: "health_check extension enabled should return probe",
823+
config: &Config{
824+
Extensions: &AnyConfig{
825+
Object: map[string]interface{}{
826+
"health_check": map[string]interface{}{},
827+
},
828+
},
829+
Service: Service{
830+
Extensions: []string{"health_check"},
831+
},
832+
},
833+
wantProbe: &v1.Probe{
834+
ProbeHandler: v1.ProbeHandler{
835+
HTTPGet: &v1.HTTPGetAction{
836+
Path: "/",
837+
Port: intstr.FromInt32(13133),
838+
},
839+
},
840+
},
841+
},
842+
{
843+
name: "health_check extension with custom path",
844+
config: &Config{
845+
Extensions: &AnyConfig{
846+
Object: map[string]interface{}{
847+
"health_check": map[string]interface{}{
848+
"path": "/healthz",
849+
},
850+
},
851+
},
852+
Service: Service{
853+
Extensions: []string{"health_check"},
854+
},
855+
},
856+
wantProbe: &v1.Probe{
857+
ProbeHandler: v1.ProbeHandler{
858+
HTTPGet: &v1.HTTPGetAction{
859+
Path: "/healthz",
860+
Port: intstr.FromInt32(13133),
861+
},
862+
},
863+
},
864+
},
865+
{
866+
name: "health_check extension with custom endpoint port",
867+
config: &Config{
868+
Extensions: &AnyConfig{
869+
Object: map[string]interface{}{
870+
"health_check": map[string]interface{}{
871+
"endpoint": "0.0.0.0:8080",
872+
},
873+
},
874+
},
875+
Service: Service{
876+
Extensions: []string{"health_check"},
877+
},
878+
},
879+
wantProbe: &v1.Probe{
880+
ProbeHandler: v1.ProbeHandler{
881+
HTTPGet: &v1.HTTPGetAction{
882+
Path: "/",
883+
Port: intstr.FromInt32(8080),
884+
},
885+
},
886+
},
887+
},
888+
{
889+
name: "extension without liveness probe should return nil",
890+
config: &Config{
891+
Extensions: &AnyConfig{
892+
Object: map[string]interface{}{
893+
"jaeger_query": map[string]interface{}{},
894+
},
895+
},
896+
Service: Service{
897+
Extensions: []string{"jaeger_query"},
898+
},
899+
},
900+
wantProbe: nil,
901+
},
902+
{
903+
name: "invalid health_check config should return error",
904+
config: &Config{
905+
Extensions: &AnyConfig{
906+
Object: map[string]interface{}{
907+
"health_check": func() {},
908+
},
909+
},
910+
Service: Service{
911+
Extensions: []string{"health_check"},
912+
},
913+
},
914+
wantErr: true,
915+
},
916+
}
917+
918+
for _, tt := range tests {
919+
t.Run(tt.name, func(t *testing.T) {
920+
got, err := tt.config.GetLivenessProbe(logr.Discard())
921+
if (err != nil) != tt.wantErr {
922+
t.Errorf("Config.GetLivenessProbe() error = %v, wantErr %v", err, tt.wantErr)
923+
return
924+
}
925+
if diff := cmp.Diff(tt.wantProbe, got); diff != "" {
926+
t.Errorf("Config.GetLivenessProbe() mismatch (-want +got):\n%s", diff)
927+
}
928+
})
929+
}
930+
}
931+
932+
func TestConfig_GetReadinessProbe(t *testing.T) {
933+
tests := []struct {
934+
name string
935+
config *Config
936+
wantProbe *v1.Probe
937+
wantErr bool
938+
}{
939+
{
940+
name: "nil extensions should return nil",
941+
config: &Config{
942+
Extensions: nil,
943+
Service: Service{
944+
Extensions: []string{},
945+
},
946+
},
947+
wantProbe: nil,
948+
},
949+
{
950+
name: "nil extensions with health_check in service extensions should return nil",
951+
config: &Config{
952+
Extensions: nil,
953+
Service: Service{
954+
Extensions: []string{"health_check"},
955+
},
956+
},
957+
wantProbe: nil,
958+
},
959+
{
960+
name: "empty extensions should return nil",
961+
config: &Config{
962+
Extensions: &AnyConfig{
963+
Object: map[string]interface{}{},
964+
},
965+
Service: Service{
966+
Extensions: []string{},
967+
},
968+
},
969+
wantProbe: nil,
970+
},
971+
{
972+
name: "empty extensions with health_check in service extensions should return probe",
973+
config: &Config{
974+
Extensions: &AnyConfig{
975+
Object: map[string]interface{}{},
976+
},
977+
Service: Service{
978+
Extensions: []string{"health_check"},
979+
},
980+
},
981+
wantProbe: &v1.Probe{
982+
ProbeHandler: v1.ProbeHandler{
983+
HTTPGet: &v1.HTTPGetAction{
984+
Path: "/",
985+
Port: intstr.FromInt32(13133),
986+
},
987+
},
988+
},
989+
},
990+
{
991+
name: "health_check extension enabled should return probe",
992+
config: &Config{
993+
Extensions: &AnyConfig{
994+
Object: map[string]interface{}{
995+
"health_check": map[string]interface{}{},
996+
},
997+
},
998+
Service: Service{
999+
Extensions: []string{"health_check"},
1000+
},
1001+
},
1002+
wantProbe: &v1.Probe{
1003+
ProbeHandler: v1.ProbeHandler{
1004+
HTTPGet: &v1.HTTPGetAction{
1005+
Path: "/",
1006+
Port: intstr.FromInt32(13133),
1007+
},
1008+
},
1009+
},
1010+
},
1011+
{
1012+
name: "health_check extension with custom path",
1013+
config: &Config{
1014+
Extensions: &AnyConfig{
1015+
Object: map[string]interface{}{
1016+
"health_check": map[string]interface{}{
1017+
"path": "/healthz",
1018+
},
1019+
},
1020+
},
1021+
Service: Service{
1022+
Extensions: []string{"health_check"},
1023+
},
1024+
},
1025+
wantProbe: &v1.Probe{
1026+
ProbeHandler: v1.ProbeHandler{
1027+
HTTPGet: &v1.HTTPGetAction{
1028+
Path: "/healthz",
1029+
Port: intstr.FromInt32(13133),
1030+
},
1031+
},
1032+
},
1033+
},
1034+
{
1035+
name: "health_check extension with custom endpoint port",
1036+
config: &Config{
1037+
Extensions: &AnyConfig{
1038+
Object: map[string]interface{}{
1039+
"health_check": map[string]interface{}{
1040+
"endpoint": "0.0.0.0:8080",
1041+
},
1042+
},
1043+
},
1044+
Service: Service{
1045+
Extensions: []string{"health_check"},
1046+
},
1047+
},
1048+
wantProbe: &v1.Probe{
1049+
ProbeHandler: v1.ProbeHandler{
1050+
HTTPGet: &v1.HTTPGetAction{
1051+
Path: "/",
1052+
Port: intstr.FromInt32(8080),
1053+
},
1054+
},
1055+
},
1056+
},
1057+
{
1058+
name: "extension without readiness probe should return nil",
1059+
config: &Config{
1060+
Extensions: &AnyConfig{
1061+
Object: map[string]interface{}{
1062+
"jaeger_query": map[string]interface{}{},
1063+
},
1064+
},
1065+
Service: Service{
1066+
Extensions: []string{"jaeger_query"},
1067+
},
1068+
},
1069+
wantProbe: nil,
1070+
},
1071+
{
1072+
name: "invalid health_check config should return error",
1073+
config: &Config{
1074+
Extensions: &AnyConfig{
1075+
Object: map[string]interface{}{
1076+
"health_check": func() {},
1077+
},
1078+
},
1079+
Service: Service{
1080+
Extensions: []string{"health_check"},
1081+
},
1082+
},
1083+
wantErr: true,
1084+
},
1085+
}
1086+
1087+
for _, tt := range tests {
1088+
t.Run(tt.name, func(t *testing.T) {
1089+
got, err := tt.config.GetReadinessProbe(logr.Discard())
1090+
if (err != nil) != tt.wantErr {
1091+
t.Errorf("Config.GetReadinessProbe() error = %v, wantErr %v", err, tt.wantErr)
1092+
return
1093+
}
1094+
if diff := cmp.Diff(tt.wantProbe, got); diff != "" {
1095+
t.Errorf("Config.GetReadinessProbe() mismatch (-want +got):\n%s", diff)
1096+
}
1097+
})
1098+
}
1099+
}

0 commit comments

Comments
 (0)