Native embed attempt

[MattSurabian]

This is an attempt to handle #3638 and replace go-bindata with native embeds. The few naming changes and restructuring done here are not fully thought out, but figured this was enough to at least see if the approach is decent.

The ceb and cli initial config generation pieces seem to work without much issue but whether or not it's advisable to keep the same asset interface that go-bindata used is up for debate.

The ui asset loading portion works but there are issues with how make docker/server is using the newly created UI module that I believe would be solved once this was merged but there could be issues with that approach. The approach here also would impact CI jobs that build and publish static assets because the embded code no longer lives in the server's gen package. I haven't updated any of the circle CI config relating to static UI assets in this PR.

What's included here does seem to build and correctly serve the ceb, initial configuration, and the UI when just using the make bin commands and working with the binary directly. Not sure if the implications on the changes to the UI actually make sense in the broader context of maintaining waypoint. It may be better to utilize embed's differently so that the generated embeded assets end up in the server's gen package like before but limitations on embed directives not applying to parent directories makes that tricky.

Happy to change the approach or abandon this entirely; curious to hear feedback anyway.

[hashicorp-cla]

All committers have signed the CLA.

[catsby]

Hey @MattSurabian ! I apologize for the silence, I offered to pick this up on our side and got pulled away unexpectedly. I'm starting to review this now, myself having some prior experience doing this swap, but I wanted to set some expectations that it may take a few more days to fully review. Specifically, you mention some rough edges or gotchas that we need to think through, so we may see some iterations on this.

Regardless thank you for kicking this off! I'm going to evaluate it as-is for now and then spend some time thinking and discussing the items you brought up with the team internally. We'll be in touch, and thanks again!

[MattSurabian]

Thanks @catsby, no sweat on the silence take your time. I just had some free cycles after Hashiconf and ran into the whole go-bindata fork drama when checking this project out for the first time. I've done a similar thing a few times before so thought I'd take a stab at it. Feel free to do whatever you think is best here. Happy to iterate on it with guidance, work together on it, or just toss it out in favor of a someone else's totally different approach. Have a good weekend!

[MattSurabian]

Rebased latest main, resolved conflicts, and force pushed. Still no rush on this PR, I just noticed the conflicts and figured better to address em.

[catsby]

Hey @MattSurabian - I'm having a bit of trouble trying to get things to compile on my end. I found I had to make a directory ui/dist(?) and also delete internal/assets/prod.go. After that, make bin did work but things like make docker/server do not. I know you mention that in your PR but I'd like to see if we could address that before merging. Are there any other steps you had to take to get this compiling? Are you running the server directly?

[MattSurabian]

Ah, I haven't actually tried to recompile since I rebased in the latest main. I didn't have to make that directory or do anything else special. Let me give this another check over the weekend and maybe we can connect next week about the docker thing? I was running the server binary directly.

…

On Fri, Nov 4, 2022, 17:39 Clint ***@***.***> wrote: Hey @MattSurabian <https://github.com/MattSurabian> - I'm having a bit of trouble trying to get things to compile on my end. I found I had to make a directory ui/dist(?) and also delete internal/assets/prod.go. After that, make bin did work but things like make docker/server do not. I know you mention that in your PR but I'd like to see if we could address that before merging. Are there any other steps you had to take to get this compiling? Are you running the server directly? — Reply to this email directly, view it on GitHub <#4014 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAIU5NCQ7WVCLIRC73R5SS3WGV7AXANCNFSM6AAAAAARALS23U> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[MattSurabian]

Ok, so I've rebased in the latest main (this time making sure to fully delete pkg/server/gen/bindata_ui.go instead of resolving the conflict with it left as an empty file) and have some info regarding building this branch as there are a few gotchas when coming from the go-bindata setup.

• If you're running on an old checkout where you have compiled before there are git ignored go-bindata generated files that must be explicitly deleted: internal/assets/prod.go or go will complain about duplicate definitions of embeded content.
• If you're compiling for the first time and you have not yet generated the front-end assets it will error on the missing ui/dist folder. There was a pre-existing comment in the Makefile which I think is lost in this branch https://github.com/hashicorp/waypoint/blob/main/Makefile#L159 Essentially you can't embed something that doesn't exist. This likely needs to be solved in the workflow by requiring the front end assets are built before everything else or committed (yuck)

[MattSurabian]

Regarding the make docker/server build issue... This is happening because at build time the docker container does not have the UI folder (it's ignored in .dockerignore https://github.com/hashicorp/waypoint/blob/main/.dockerignore#L4 and as such isn't passed in here https://github.com/hashicorp/waypoint/blob/main/Dockerfile#L20) and so as a result go is trying to fetch it from the module cache. The mod cache only knows about the main branch where the go file doesn't yet exist.

To allow the make docker/server command to function I've opted to remove that line from the .dockerignore file for now but as with a few other things in this PR it's not clear if that's the best approach; unfortunately it may be the only option if we opt to keep the ui go module located in that ui folder due to the limitations on go's native embed functionality traversing the directory tree.

[MattSurabian]

So I was thinking about the transition away from the existing embed calls and wondering if the cleanup should be a distinct Make task that could be added to the existing build task if this is merged and then removed at some later time?

Might make the shift away from bindata less disruptive.

[MattSurabian]

@catsby I have some time to pick this up again and will resolve these conflicts. Wondering if you have thoughts on the comments I left in November?

[MattSurabian]

@catsby Due to the files this PR touches it is perpetually in conflict. Let me know if yall want to go a different direction with this or if you think the Make approach suggested above is worth an attempt as it has been a minute since I've checked in on this PR.

[catsby]

Hey @MattSurabian! I sincerely apologize for the weeks/months of silence here. I think the best course here is to close this PR for now. I honestly appreciate the work you did here and I'm very sorry for not following up and providing the feedback needed to see this through. I'm grateful for your contribution (attempts here, at least) and again very sorry for not being responsive to you here.

For now though I am going to close this. Thank you again.