Fixed agency access #153

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Merged

ewoutp merged 2 commits into master from agency-redirect-fix

Apr 26, 2018

Contributor

ewoutp commented Apr 26, 2018 •

edited

Loading

This PR solves the problem that the starter could not talk the the agency (in 100% of the cases) when authentication was turned on.

ewoutp added 2 commits

April 26, 2018 16:14


          Fixed agency access

c8fe98e


          Removed commented-out code

488de21

mchacki suggested changes

View reviewed changes

mchacki left a comment

One question on 6 places (same question)

Manual testing of the issue works as expected.

service/peer.go

    
              		scheme := NewURLSchemes(p.IsSecure).Browser

              		ep := fmt.Sprintf("%s://%s", scheme, net.JoinHostPort(p.Address, strconv.Itoa(port)))

              		c, err := clientBuilder([]string{ep}, false)

              		c, err := clientBuilder([]string{ep}, ConnectionTypeDatabase)

mchacki Apr 26, 2018

This now has a different value for DontFollowRedirects is this correct?

service/peer.go

    
              		scheme := NewURLSchemes(p.IsSecure).Browser

              		ep := fmt.Sprintf("%s://%s", scheme, net.JoinHostPort(p.Address, strconv.Itoa(port)))

              		c, err := clientBuilder([]string{ep}, false)

              		c, err := clientBuilder([]string{ep}, ConnectionTypeDatabase)

mchacki Apr 26, 2018

This now has a different value for DontFollowRedirects is this correct?

service/upgrade_manager.go

    
              	// Check all

              	for _, ep := range endpoints {

              		c, err := m.upgradeManagerContext.CreateClient([]string{ep}, false)

              		c, err := m.upgradeManagerContext.CreateClient([]string{ep}, ConnectionTypeDatabase)

mchacki Apr 26, 2018

This now has a different value for DontFollowRedirects is this correct?

service/upgrade_manager.go

    
              	// Check all

              	for _, ep := range endpoints {

              		c, err := m.upgradeManagerContext.CreateClient([]string{ep}, false)

              		c, err := m.upgradeManagerContext.CreateClient([]string{ep}, ConnectionTypeDatabase)

mchacki Apr 26, 2018

This now has a different value for DontFollowRedirects is this correct?

service/upgrade_manager.go

    
              	// Check all

              	for _, ep := range endpoints {

              		c, err := m.upgradeManagerContext.CreateClient([]string{ep}, false)

              		c, err := m.upgradeManagerContext.CreateClient([]string{ep}, ConnectionTypeDatabase)

mchacki Apr 26, 2018

This now has a different value for DontFollowRedirects is this correct?

service/upgrade_manager.go

    
              		groupVersions := make([]string, len(endpoints))

              		for i, ep := range endpoints {

              			c, err := m.upgradeManagerContext.CreateClient([]string{ep}, false)

              			c, err := m.upgradeManagerContext.CreateClient([]string{ep}, ConnectionTypeDatabase)

mchacki Apr 26, 2018

This now has a different value for DontFollowRedirects is this correct?

Contributor Author

ewoutp commented Apr 26, 2018

The answer is yes to all, allthough I'll do 1 more check for activefailover.

For everything except agents, the follow-redirect is irrelevant because the servers will not yield a 307.

ewoutp merged commit aa3fd3e into master

ewoutp deleted the agency-redirect-fix branch

April 26, 2018 15:28

mchacki commented Apr 26, 2018

i think you are right. 307 is never created by DBServers.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet