Add mTLS auth credential support

[rainest]

Add support for mTLS auth to decK.

• This needs a proper go-kong release to use; currently it's using master as no go-kong release contains the mTLS code yet.
• This can't actually sync existing config (it creates duplicates) because of limitations in the resource.

The mtls-auth credentials don't have unique identifiers other than the ID, so we cannot find existing matching resources. Although not great, the simplest strategy I can think of to handle this is to tack CA Certificates in the controller: we just require that you provide an ID for all mtls-auth credentials. It's not particularly user-friendly, but is probably the best option barring major changes in the DAO to allow multi-field uniques. That isn't in this PR yet, as I think we should review other options as a team before moving on. Draft as such.

[rainest]

This is a bit better. However, I'm not sure how to fix the codegen stuff that makes that CI test fail (doesn't look like there's a generator script) and there's some oddness with the error printing, as discussed below.

Given https://gist.github.com/rainest/7ba9bc610d8b7a2bf663feb11cc0304f#file-bad-yaml (which contains a no-ID mtls-auth) deck prints:

Error: mtls-auth for Consumer '1042cbd2-85ba-4f19-a51c-a8fde8d0a594' with SubjectName 'example.com' lacks ID

I'm missing something though. https://github.com/rainest/deck/blob/275bffc/file/builder.go#L397-L412 should print the username if present, custom ID if not, and ID if neither (which shouldn't happen--it's invalid). However, this apparently gives us a consumer with neither a username nor custom ID:

consumers:
- username: foo
  mtlsauth_credentials:
  - subject_name: example.com

That is indeed what we get in code, but I'm not sure why:

$ dlv debug -- sync -s /tmp/bad.yaml
Type 'help' for list of commands.
(dlv) b /home/traines/src/deck/file/builder.go:401
Breakpoint 1 set at 0xa08d29 for github.com/hbagdi/deck/file.(*stateBuilder).ingestMTLSAuths() ./file/builder.go:401
(dlv) c
> github.com/hbagdi/deck/file.(*stateBuilder).ingestMTLSAuths() ./file/builder.go:401 (hits goroutine(7):1 total:1) (PC: 0xa08d29)
   396:			cred := cred
   397:			if utils.Empty(cred.ID) {
   398:				// normally, we'd want to look up existing resources in this case
   399:				// however, this is impossible here: mtls-auth simply has no unique fields other than ID,
   400:				// so we just give up and complain about it
=> 401:				var consumerFriendlyName *string
   402:				if !utils.Empty(cred.Consumer.Username) {
   403:					consumerFriendlyName = cred.Consumer.Username
   404:				} else if !utils.Empty(cred.Consumer.CustomID) {
   405:					consumerFriendlyName = cred.Consumer.CustomID
   406:				} else {
(dlv) p cred.Consumer.Username
*string nil
(dlv) p cred.Consumer
*github.com/hbagdi/go-kong/kong.Consumer {
	ID: *"91e7466e-448e-48b3-bbde-ca67cff1bdd3",
	CustomID: *string nil,
	Username: *string nil,
	CreatedAt: *int64 nil,
	Tags: []*string len: 0, cap: 0, nil,}

[hbagdi]

However, I'm not sure how to fix the codegen stuff that makes that CI test fail (doesn't look like there's a generator script)

Run go generate ./.... This project uses go toolchain for almost everything, although that's not documented.
You are looking for https://github.com/hbagdi/deck/blob/master/file/codegen/main.go and https://github.com/hbagdi/deck/blob/master/file/types.go#L477. Please read up on generate tag in Go to get a more thorough understanding.

I'm missing something though. https://github.com/rainest/deck/blob/275bffc/file/builder.go#L397-L412 should print the username if present, custom ID if not, and ID if neither (which shouldn't happen--it's invalid). However, this apparently gives us a consumer with neither a username nor custom ID:

Trace the path from start to the point where you get in the code. It is a little round about.
Let's look at this snippet borrowed from cmd/common.go:

// get the current state from kong's db
	rawState, err := dump.Get(client, dumpConfig)
	if err != nil {
		return err
	}
// create an in-memory representation of the state with all the indexes
	currentState, err := state.Get(rawState)
	if err != nil {
		return err
	}

	// read the target state from the file and then match it against the current state we read from the admin api, this is where
// a lot of magic happens, more below
	rawState, err = file.Get(targetContent, file.RenderConfig{
		CurrentState: currentState,
		KongVersion:  kongVersion,
	})
	if err != nil {
		return err
	}
// once we have read the state in a flat struct, construct the necessary indices from it
	targetState, err := state.Get(rawState)
	if err != nil {
		return err
	}

// now run the diff since we have both the structs
	s, _ := diff.NewSyncer(currentState, targetState)
	stats, errs := solver.Solve(stopChannel, s, client, parallelism, dry)

At this point in writing this comment, I realized that I was explaining the wrong thing to you.

You are looking for https://github.com/rainest/deck/blob/275bffc/file/builder.go#L256. That line sets the consumer on the mtls-auth entity and as we see, that only contains an ID.

praise: I am so happy to see you make the effort to provide a helpful error to the end user in such cases, but this is a little difficult to do so right now with the approach you are taking. Try generating the schema and then run the bad.yaml through that. It will error out with some more context and that should be better. It is not perfect but should be okay.

[rainest]

Didn't see any differences after schema generation (other than clearing the CI failure) but it's easy enough, if a bit cumbersome, to just pass the consumer details in, complain about them if needed, and strip them after when actually creating the resource.

With that I think we're good to go (famous last words)!

[hbagdi]

Reviewed half the code, will continue again as I find time. This is looking pretty good overall.

[hbagdi]

I'll do a go-kong release before a final deck release to take care of this. This works as it is for this change set.

[rainest]

I'm missing something in the tests/deck code around handling around handling no-tag entities. The actual want definition has no tag: https://github.com/rainest/deck/blob/feat/mtls-auth/file/builder_test.go#L962-L967

However, that runs into this issue:

=== RUN   Test_stateBuilder_consumers/matches_ID_of_an_existing_credential
> github.com/hbagdi/deck/file.Test_stateBuilder_consumers.func1() ./builder_test.go:1208 (hits goroutine(38):1 total:4) (PC: 0x9fb2c1)
	tt.name: "matches ID of an existing credential"
Warning: listing may not match stale executable
  1203:					kongVersion:   kong140Version,
  1204:				}
  1205:				if tt.fields.kongVersion != nil {
  1206:					b.kongVersion = *tt.fields.kongVersion
  1207:				}
=>1208:				d, _ := utils.GetKongDefaulter()
  1209:				b.defaulter = d
  1210:				b.build()
  1211:				assert.Equal(tt.want, b.rawState)
  1212:			})
  1213:		}
(dlv) p tt.want.MTLSAuths
[]*github.com/hbagdi/go-kong/kong.MTLSAuth len: 1, cap: 1, [
	*{
		Consumer: *(*"github.com/hbagdi/go-kong/kong.Consumer")(0xc00040d580),
		CreatedAt: *int nil,
		ID: *"533c259e-bf71-4d77-99d2-97944c70a6a4",
		SubjectName: *"test@example.com",
		CACertificate: *github.com/hbagdi/go-kong/kong.CACertificate nil,
		Tags: []*string len: 1, cap: 1, [
			*"tag1",
		],},
]

So I'm guessing something related to https://github.com/rainest/deck/blob/feat/mtls-auth/file/builder_test.go#L970-L972 adds it, but I'm not sure how to correctly remove that--I can probably find it eventually but figure it will be quicker to ask in this case. cfa281b makes the test pass, but is almost certainly not the correct way to do so.

[hbagdi]

It took me longer than I would like to admit to track this down:

Following patch on top of your branch fixes the issue:

diff --git a/file/builder_test.go b/file/builder_test.go
index a6b5744..025f2fb 100644
--- a/file/builder_test.go
+++ b/file/builder_test.go
@@ -1052,7 +1052,6 @@ func Test_stateBuilder_consumers(t *testing.T) {
                                                Consumer: &kong.Consumer{
                                                        ID: kong.String("4bfcb11f-c962-4817-83e5-9433cf20b663"),
                                                },
-                                               Tags: kong.StringSlice("tag1"),
                                        },
                                },
                        },
@@ -1193,8 +1192,6 @@ func Test_stateBuilder_consumers(t *testing.T) {
                        },
                },
        }
-       var empty []*string
-       tests[3].want.MTLSAuths[0].Tags = empty
        for _, tt := range tests {
                t.Run(tt.name, func(t *testing.T) {
                        b := &stateBuilder{

[hbagdi]

I applied the above patch and a bunch others and I was ready to merge but the code didn't work when I tested manually.

The test I performed had two mtls-auth creds configured:

{
    "data": [
        {
            "ca_certificate": null,
            "consumer": {
                "id": "c1d3008b-7149-4da3-a690-0028b6e74043"
            },
            "created_at": 1596081369,
            "id": "b3977502-ccd1-4fd7-a30c-c184b8d48676",
            "subject_name": "yolo42.com"
        },
        {
            "ca_certificate": {
                "id": "c402ef49-f07d-4361-bc5d-87be76ccec4e"
            },
            "consumer": {
                "id": "c1d3008b-7149-4da3-a690-0028b6e74043"
            },
            "created_at": 1596081425,
            "id": "c73327d4-22b8-43a5-8521-94c154f054a3",
            "subject_name": "yolo42.com"
        }
    ],
    "next": null
}

When I run deck dump, it fails with:

Error: building state: inserting mtls-auth into state: entity already exists

I've pushed the updated patch here: 9097da7

Please base your work on top of that patch now (force push on this PR) and also resolve the problem described above.

[rainest]

mtls-auth's lack of unique fields strikes again. the fix in b9b5c54 is silly, but whatever, it works 🤷

[hbagdi]

Merged manually b1b6633