Skip to content
This repository has been archived by the owner on Nov 5, 2021. It is now read-only.

Commit

Permalink
63729 a23134379909 bfca3 ac3 c378 b48 (#452)
Browse files Browse the repository at this point in the history
* Fix a bug in how we check for more than one extension in a probe config.

Use proto.ExtensionDescs(msg) to get extension descriptors instead of RegisteredExtensions(msg). Former returns extensions from the proto message, while the latter returns all extensions from the "type" of the proto message.

See #450 for more details.

PiperOrigin-RevId: 329415541
  • Loading branch information
manugarg authored Sep 1, 2020
1 parent 4bdceab commit f513f79
Show file tree
Hide file tree
Showing 6 changed files with 242 additions and 79 deletions.
23 changes: 8 additions & 15 deletions probes/probes.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ package probes

import (
"context"
"errors"
"fmt"
"regexp"
"sync"
Expand Down Expand Up @@ -87,29 +86,23 @@ type ProbeInfo struct {
}

func getExtensionProbe(p *configpb.ProbeDef) (Probe, interface{}, error) {
extensions := proto.RegisteredExtensions(p)
if len(extensions) > 1 {
return nil, nil, fmt.Errorf("only one probe extension is allowed per probe, got %d extensions", len(extensions))
}
var field int
var desc *proto.ExtensionDesc
// There should be only one extension.
for f, d := range extensions {
field = int(f)
desc = d
extensions, err := proto.ExtensionDescs(p)
if err != nil {
return nil, nil, fmt.Errorf("error getting extensions from the probe config (%s): %v", p.String(), err)
}
if desc == nil {
return nil, nil, errors.New("no probe extension in probe config")
if len(extensions) != 1 {
return nil, nil, fmt.Errorf("there should be exactly one extension in the probe config (%s), got %d extensions", p.String(), len(extensions))
}
desc := extensions[0]
value, err := proto.GetExtension(p, desc)
if err != nil {
return nil, nil, err
}
extensionMapMu.Lock()
defer extensionMapMu.Unlock()
newProbeFunc, ok := extensionMap[field]
newProbeFunc, ok := extensionMap[int(desc.Field)]
if !ok {
return nil, nil, fmt.Errorf("no probes registered for the extension: %d", field)
return nil, nil, fmt.Errorf("no probes registered for the extension: %d", desc.Field)
}
return newProbeFunc(), value, nil
}
Expand Down
129 changes: 106 additions & 23 deletions probes/testdata/testdata.pb.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

7 changes: 6 additions & 1 deletion probes/testdata/testdata.proto
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,14 @@ import "github.com/google/cloudprober/probes/proto/config.proto";
option go_package = "github.com/google/cloudprober/probes/testdata";

message FancyProbe {
required string name = 1;
optional string name = 1;
}

message AnotherFancyProbe {
optional string name = 1;
}

extend probes.ProbeDef {
optional FancyProbe fancy_probe = 200;
optional AnotherFancyProbe another_fancy_probe = 201;
}
24 changes: 9 additions & 15 deletions targets/targets.go
Original file line number Diff line number Diff line change
Expand Up @@ -361,30 +361,24 @@ func New(targetsDef *targetspb.TargetsDef, ldLister lameduck.Lister, globalOpts
}

func getExtensionTargets(pb *targetspb.TargetsDef, l *logger.Logger) (Targets, error) {
extensions := proto.RegisteredExtensions(pb)
if len(extensions) > 1 {
return nil, fmt.Errorf("only one extension is allowed per targets definition, got %d extensions", len(extensions))
}
var field int
var desc *proto.ExtensionDesc
// There should be only one extension in one protobuf message.
for f, d := range extensions {
field = int(f)
desc = d
extensions, err := proto.ExtensionDescs(pb)
if err != nil {
return nil, fmt.Errorf("error getting extensions from the target config (%s): %v", pb.String(), err)
}
if desc == nil {
return nil, errors.New("unrecognized target type")
if len(extensions) != 1 {
return nil, fmt.Errorf("there should be exactly one extension in the targets config (%s), got %d extensions", pb.String(), len(extensions))
}
desc := extensions[0]
value, err := proto.GetExtension(pb, desc)
if err != nil {
return nil, err
}
l.Infof("Extension field: %d, value: %v", field, value)
l.Infof("Extension field: %d, value: %v", desc.Field, value)
extensionMapMu.Lock()
defer extensionMapMu.Unlock()
newTargetsFunc, ok := extensionMap[field]
newTargetsFunc, ok := extensionMap[int(desc.Field)]
if !ok {
return nil, fmt.Errorf("no targets type registered for the extension: %d", field)
return nil, fmt.Errorf("no targets type registered for the extension: %d", desc.Field)
}
return newTargetsFunc(value, l)
}
Expand Down
Loading

0 comments on commit f513f79

Please sign in to comment.