ensure final dot on node name #2503
Conversation
|
I've approved to all tests will run, lets see how that goes and if something more needs to be updated. Can you add a changelog entry? |
|
I see that there failures in TestResolveMagicDNS, postgres) (pull_request), I imagine that is related, do you have the opportunity to take a look at those? |
|
Yeah, I noticed that as well. I am currently trying to run the integration tests locally, so that I can verify my changes. Will look into it. |
49ca0de to
18913c6
Compare
18913c6 to
0741e0b
Compare
|
I did some change to the |
|
Is there anything I can help here with? I am not sure the |
kradalby
left a comment
There was a problem hiding this comment.
looks good, you can ignore TestValidateResolvConf
| [#2493](https://github.com/juanfont/headscale/pull/2493) | ||
| - If a OIDC provider doesn't include the `email_verified` claim in its ID | ||
| tokens, Headscale will attempt to get it from the UserInfo endpoint. | ||
| - node FQDNs in the netmap will now contain a dot (".") at the end. This aligns |
|
There is a couple of |
I'll have a look. |
Pull Request Revisions
|
|
I fixed the I also had a look into the failing |
| type buffer struct { | ||
| buffer bytes.Buffer | ||
| mutex sync.Mutex | ||
| } | ||
|
|
||
| // Write appends the contents of p to the buffer, growing the buffer as needed. It returns | ||
| // the number of bytes written. | ||
| func (s *buffer) Write(p []byte) (n int, err error) { | ||
| s.mutex.Lock() | ||
| defer s.mutex.Unlock() | ||
| return s.buffer.Write(p) | ||
| } | ||
|
|
||
| // String returns the contents of the unread portion of the buffer | ||
| // as a string. If the Buffer is a nil pointer, it returns "<nil>". | ||
| func (s *buffer) String() string { | ||
| s.mutex.Lock() | ||
| defer s.mutex.Unlock() | ||
| return s.buffer.String() | ||
| } |
There was a problem hiding this comment.
The error message in String() method indicates it will return "" if the Buffer is a nil pointer, but the implementation doesn't handle this case. Consider adding a nil check at the beginning of the method or removing this comment if nil buffers aren't expected in your usage pattern.
This ensures that nodes which have a base domain set, will have a dot appended to their FQDN. Resolves: juanfont#2501
Waiting a bit more than the TTL of the OIDC token seems to remove some flakiness of this test. This furthermore makes use of a go func safe buffer which should avoid race conditions.
Hi @kradalby , do you have any opinion on this? Or should this just be left as it is? |
sorry I've been swamped, I think it sounds fine for now, I cant recall any specific client changes, but we will likely drop support for some of those clients with our 10 last client goal, so lets not spend that much effort on it for now. If the tests (even with reruns) passes now, this one should be good. |
|
A couple of months ago, I built my private support for After patching my changes to headscale v0.26.0,
After reverting this change, |
This ensures that nodes which have a base domain set, will have a dot appended to their FQDN. This way, nodes in a netmap will look the same as they do when using tailscale.com services.
Fixes: #2501