[targets] Make targets optional for certain probes (cloudprober#614)

- UserDefined, External and Extensions probe types don't necessarily need targets.
jumpojoy · Oct 31, 2023 · 3faa199 · 3faa199
1 parent d290713
commit 3faa199
Show file tree

Hide file tree

Showing 5 changed files with 80 additions and 6 deletions.
diff --git a/probes/options/options.go b/probes/options/options.go
@@ -167,6 +167,16 @@ func BuildProbeOptions(p *configpb.ProbeDef, ldLister endpoint.Lister, globalTar
 		Logger:            logger.NewWithAttrs(slog.String("probe", p.GetName())),
 	}
 
+	if p.GetTargets() == nil {
+		if p.GetType() != configpb.ProbeDef_USER_DEFINED && p.GetType() != configpb.ProbeDef_EXTERNAL && p.GetType() != configpb.ProbeDef_EXTENSION {
+			return nil, fmt.Errorf("targets requied for probe type: %s", p.GetType().String())
+		} else {
+			p.Targets = &targetspb.TargetsDef{
+				Type: &targetspb.TargetsDef_DummyTargets{},
+			}
+		}
+	}
+
 	if opts.Targets, err = targets.New(p.GetTargets(), ldLister, globalTargetsOpts, l, opts.Logger); err != nil {
 		return nil, err
 	}

diff --git a/probes/options/options_test.go b/probes/options/options_test.go
@@ -424,3 +424,64 @@ func TestRecordMetrics(t *testing.T) {
 		})
 	}
 }
+
+func TestNilTargets(t *testing.T) {
+	tests := []struct {
+		cfg           *configpb.ProbeDef
+		wantEndpoints []endpoint.Endpoint
+		wantErr       bool
+	}{
+		{
+			cfg: &configpb.ProbeDef{
+				Type: configpb.ProbeDef_PING.Enum(),
+				Name: proto.String("test-probe"),
+			},
+			wantErr: true,
+		},
+		{
+			cfg: &configpb.ProbeDef{
+				Type: configpb.ProbeDef_USER_DEFINED.Enum(),
+				Name: proto.String("test-probe"),
+			},
+			wantEndpoints: []endpoint.Endpoint{{Name: ""}},
+		},
+		{
+			cfg: &configpb.ProbeDef{
+				Type: configpb.ProbeDef_EXTERNAL.Enum(),
+				Name: proto.String("test-probe"),
+			},
+			wantEndpoints: []endpoint.Endpoint{{Name: ""}},
+		},
+		{
+			cfg: &configpb.ProbeDef{
+				Type: configpb.ProbeDef_EXTENSION.Enum(),
+				Name: proto.String("test-probe"),
+			},
+			wantEndpoints: []endpoint.Endpoint{{Name: ""}},
+		},
+		{
+			cfg: &configpb.ProbeDef{
+				Type: configpb.ProbeDef_EXTERNAL.Enum(),
+				Name: proto.String("test-probe"),
+				Targets: &targetspb.TargetsDef{
+					Type: &targetspb.TargetsDef_HostNames{HostNames: "testHost"},
+				},
+			},
+			wantEndpoints: []endpoint.Endpoint{{Name: "testHost"}},
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.cfg.String(), func(t *testing.T) {
+			got, err := BuildProbeOptions(tt.cfg, nil, nil, nil)
+			if (err != nil) != tt.wantErr {
+				t.Errorf("BuildProbeOptions() error = %v, wantErr %v", err, tt.wantErr)
+				return
+			}
+			if err != nil {
+				return
+			}
+			assert.Equal(t, tt.wantEndpoints, got.Targets.ListEndpoints())
+		})
+	}
+}
diff --git a/probes/proto/config.pb.go b/probes/proto/config.pb.go
diff --git a/probes/proto/config.proto b/probes/proto/config.proto
@@ -64,8 +64,9 @@ message ProbeDef {
   // Default timeout is 1s.
   optional string timeout = 17;
 
-  // Targets for the probe
-  required targets.TargetsDef targets = 6;
+  // Targets for the probe. Targets are required for all probes except
+  // for external, user_defined, and extension probe types.
+  optional targets.TargetsDef targets = 6;
 
   // Latency distribution. If specified, latency is stored as a distribution.
   optional metrics.Dist latency_distribution = 7;

diff --git a/probes/proto/config_proto_gen.cue b/probes/proto/config_proto_gen.cue
@@ -75,7 +75,8 @@ import (
 	// Default timeout is 1s.
 	timeout?: string @protobuf(17,string)
 
-	// Targets for the probe
+	// Targets for the probe. Targets are required for all probes except
+	// for external, user_defined, and extension probe types.
 	targets?: proto.#TargetsDef @protobuf(6,targets.TargetsDef)
 
 	// Latency distribution. If specified, latency is stored as a distribution.