Detect Breaking proto changes

Gloo Edge Product

Open Source

Gloo Edge Version

Any

Is your feature request related to a problem? Please describe.

While updating protos, a message type can be defined as a separate message or as a nested one.

When a field is defined in a message, type name resolution in the protocol buffer language works like C++: first, the innermost scope is searched, then the next-innermost, and so on. (Ref: https://protobuf.dev/programming-guides/proto3/#nested)

Using Nested Messages

`Bar` defined as a nested message

message Foo
{
  # It first searches for the definition of Bar within Foo.
  # It identifies Bar defined within the scope of Foo itself
  Bar nested_bar = 1; 

  message Bar 
  {
     int32 inside = 1;
  }
}

# On conversion to yaml we get the following
Foo:
  properties:
    nestedBar:
      properties:
        inside:            # It picks up the field "inside" from .Foo.Bar
          format: int32
          type: integer
      type: object
  type: object

# On generating the .pb.go we get the following
type Foo struct {
    # The type of nestedBar is `Foo_Bar` indicating that 
    # it has picked up the definition of Bar within Foo
    NestedBar *Foo_Bar    
}

`Bar` is defined multiple times with different scopes

message Bar {
  int32 outside = 1;
}

message Foo
{
  # It identifies the type of Bar as defined within the scope of Foo 
  # ie: .Foo.Bar
  Bar nested_bar = 1;

  message Bar 
  {
    int32 inside = 1;
  }
}  

# On conversion to yaml we get the following
Foo:
  properties:
    nestedBar:
      properties:
        inside:            # It picks up the field "inside" from .Foo.Bar
          format: int32
          type: integer
      type: object
  type: object

# On generating the .pb.go we get the following
type Foo struct {
    # The type of nestedBar is `Foo_Bar` indicating that 
    # it has picked up the definition of Bar within Foo
    NestedBar *Foo_Bar    
}

Using Separate Messages

`Bar` defined as a separate message

message Bar 
{
   int32 outside = 1;
}

message Foo
{
  # It first searches for the definition of Bar within Foo.
  # Since Bar is not found within the scope of Foo
  # it searches one level up and identifies Bar
  Bar nested_bar = 1; 
}

# On conversion to yaml we get the following
Foo:
  properties:
    nestedBar:
      properties:
        outside:            # It picks up the field "outside" from .Bar
          format: int32
          type: integer
      type: object
  type: object

# On generating the .pb.go we get the following
type Foo struct {
    # The type of nestedBar is `Bar`
    NestedBar *Bar         
}

The Problem

Building on from the previous example: Bar has already been defined and used as the type for nested_bar within Foo. Now if Bar is re-defined within Foo, the local Bar type is now taken as the type for nestedBar since it has a local scope.

Looking at the diff we get the following :

message Bar 
{
   int32 outside = 1;
}

message Foo
{
  Bar nested_bar = 1; 

+  message Bar 
+  {
+    int32 inside = 1;
+  }
}

# On conversion to yaml we get the following
Foo:
  properties:
    nestedBar:
      properties:
-        outside:            # Change in the properties as now the local Bar
+        inside:             # message is used as the type of nestedBar
          format: int32
          type: integer
      type: object
  type: object

# On generating the .pb.go we get the following
type Foo struct {
-    NestedBar *Bar         # Change in the type of NestedBar as now the local Bar
+    NestedBar *Foo_Bar     # message is used as the type of nestedBar    
}

To a reviewer, the addition of the newly defined Bar might not raise any concerns, but the impact is that the type of nestedBar has now been changed and is a breaking change. This can be clearly seen in the generated code.

The Bigger Problem

This can become an even bigger issue if the fields in the newly defined Bar are exactly the same as the existing Bar type as there will be no changes in the yaml / CRDs. This is still a breaking change and can be seen in the generated go code.

message Bar 
{
   int32 outside = 1;
}

message Foo
{
  Bar nested_bar = 1; 

+  message Bar 
+  {
+    int32 outside = 1;
+  }     
}

# On conversion to yaml there is NO change
Foo:
  properties:
    nestedBar:
      properties:
        outside: 
          format: int32
          type: integer
      type: object
  type: object

# The break can be seen only in the generated go code
type Foo struct {
-    NestedBar *Bar        
+    NestedBar *Foo_Bar    
}

Describe the solution you'd like

Add a check for breaking proto changes, especially with messages with different scopes

Use buf breaking to check for changes in proto field types

$ buf breaking bad --against good

bad/a.proto:10:3:Field "1" on message "Foo" changed type from "Bar" to "Foo.Bar"

Describe alternatives you've considered

No response

Additional Context

This occurred in https://github.com/solo-io/gloo/pull/8495/files#diff-9a3a741756fd9cf3b5fbaceb118314b2ef89bb148958533e4cad599153d70e29R1730-R1733

solo-io / gloo