Skip to content

Commit

Permalink
fix timeout when contacting the remote auth service
Browse files Browse the repository at this point in the history
  • Loading branch information
aleoli committed Sep 10, 2021
1 parent de93ff9 commit 6c48cac
Show file tree
Hide file tree
Showing 5 changed files with 46 additions and 17 deletions.
6 changes: 5 additions & 1 deletion internal/discovery/foreign-cluster-operator/auth.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import (
"k8s.io/klog/v2"

discoveryv1alpha1 "github.com/liqotech/liqo/apis/discovery/v1alpha1"
"github.com/liqotech/liqo/internal/discovery/utils"
"github.com/liqotech/liqo/pkg/auth"
"github.com/liqotech/liqo/pkg/utils/authenticationtoken"
foreignclusterutils "github.com/liqotech/liqo/pkg/utils/foreignCluster"
Expand Down Expand Up @@ -198,7 +199,10 @@ func sendRequest(url string, payload *bytes.Buffer, insecureSkipTLSVerify bool)
tr := &http.Transport{
TLSClientConfig: &tls.Config{InsecureSkipVerify: insecureSkipTLSVerify},
}
client := &http.Client{Transport: tr}
client := &http.Client{
Transport: tr,
Timeout: utils.HTTPRequestTimeout,
}
req, err := http.NewRequestWithContext(context.TODO(), http.MethodPost, url, payload)
if err != nil {
klog.Error(err)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -146,23 +146,31 @@ func (r *ForeignClusterReconciler) Reconcile(ctx context.Context, req ctrl.Reque
}
tracer.Step("Retrieved the foreign cluster")

updateStatus := func() {
defer tracer.Step("ForeignCluster status update")
if newErr := r.Client.Status().Update(ctx, &foreignCluster); newErr != nil {
klog.Error(newErr)
err = newErr
}
}

// ------ (1) validation ------

// set labels and validate the resource spec
if cont, res, err := r.validateForeignCluster(ctx, &foreignCluster); !cont {
cont, res, err := r.validateForeignCluster(ctx, &foreignCluster)
if !cont || err != nil {
tracer.Step("Validated foreign cluster", trace.Field{Key: "requeuing", Value: true})
if err != nil {
klog.Error(err)
r.setForeignClusterStatusOnAuthUnavailable(&foreignCluster)
updateStatus()
}
return res, err
}
tracer.Step("Validated foreign cluster", trace.Field{Key: "requeuing", Value: false})

// defer the status update function
defer func() {
defer tracer.Step("ForeignCluster status update")
if newErr := r.Client.Status().Update(ctx, &foreignCluster); newErr != nil {
klog.Error(newErr)
err = newErr
}
}()
defer updateStatus()

// ensure that there are not multiple clusters with the same clusterID
if processable, err := r.isClusterProcessable(ctx, &foreignCluster); err != nil {
Expand Down Expand Up @@ -283,6 +291,14 @@ func (r *ForeignClusterReconciler) Reconcile(ctx context.Context, req ctrl.Reque
}, nil
}

func (r *ForeignClusterReconciler) setForeignClusterStatusOnAuthUnavailable(foreignCluster *discoveryv1alpha1.ForeignCluster) {
peeringconditionsutils.EnsureStatus(foreignCluster,
discoveryv1alpha1.AuthenticationStatusCondition,
discoveryv1alpha1.PeeringConditionStatusError,
"AuthNotReachable",
"The remote authentication service is not reachable")
}

// peerNamespaced enables the peering creating the resources in the correct TenantNamespace.
func (r *ForeignClusterReconciler) peerNamespaced(ctx context.Context,
foreignCluster *discoveryv1alpha1.ForeignCluster) error {
Expand Down
11 changes: 10 additions & 1 deletion internal/discovery/utils/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,19 +23,28 @@ import (
"fmt"
"io/ioutil"
"net/http"
"time"

"k8s.io/klog/v2"

"github.com/liqotech/liqo/pkg/auth"
)

const (
// HTTPRequestTimeout is the timeout used in http connection to a remote authentication service.
HTTPRequestTimeout = 5 * time.Second
)

// GetClusterInfo contacts the remote cluster to get its info,
// it returns also if the remote cluster exposes a trusted certificate.
func GetClusterInfo(skipTLSVerify bool, url string) (*auth.ClusterInfo, error) {
tr := &http.Transport{
TLSClientConfig: &tls.Config{InsecureSkipVerify: skipTLSVerify},
}
client := &http.Client{Transport: tr}
client := &http.Client{
Transport: tr,
Timeout: HTTPRequestTimeout,
}
resp, err := httpGet(context.TODO(), client, fmt.Sprintf("%s%s", url, auth.IdsURI))
if err != nil {
return nil, err
Expand Down
8 changes: 4 additions & 4 deletions pkg/liqoctl/generate/generate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -139,14 +139,14 @@ var _ = Describe("Test the generate command works as expected", func() {
commandName+" add cluster "+localClusterID+" --auth-url https://"+authEndpoint+" --id "+localClusterID+" --token "+token,
),
Entry("Default authentication service endpoint",
[]string{fmt.Sprintf("%v=%v", consts.ClusterNameParameter, clusterName)},
[]string{fmt.Sprintf("--%v=%v", consts.ClusterNameParameter, clusterName)},
commandName+" add cluster "+clusterName+" --auth-url https://"+authEndpoint+" --id "+localClusterID+" --token "+token,
),
Entry("Overridden authentication service endpoint",
[]string{
fmt.Sprintf("%v=%v", consts.ClusterNameParameter, clusterName),
fmt.Sprintf("%v=%v", consts.AuthServiceAddressOverrideParameter, "foo.bar.com"),
fmt.Sprintf("%v=%v", consts.AuthServicePortOverrideParameter, "8443"),
fmt.Sprintf("--%v=%v", consts.ClusterNameParameter, clusterName),
fmt.Sprintf("--%v=%v", consts.AuthServiceAddressOverrideParameter, "foo.bar.com"),
fmt.Sprintf("--%v=%v", consts.AuthServicePortOverrideParameter, "8443"),
},
commandName+" add cluster "+clusterName+" --auth-url https://foo.bar.com:8443 --id "+localClusterID+" --token "+token,
),
Expand Down
6 changes: 3 additions & 3 deletions pkg/liqoctl/generate/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,9 +66,9 @@ func processGenerateCommand(ctx context.Context, clientSet client.Client, liqoNa
}

// The error is discarded, since an empty string is returned in case the key is not found, which is fine.
clusterName, _ := common.ExtractValueFromArgumentList(consts.ClusterNameParameter, args)
authServiceAddressOverride, _ := common.ExtractValueFromArgumentList(consts.AuthServiceAddressOverrideParameter, args)
authServicePortOverride, _ := common.ExtractValueFromArgumentList(consts.AuthServicePortOverrideParameter, args)
clusterName, _ := common.ExtractValueFromArgumentList(fmt.Sprintf("--%v", consts.ClusterNameParameter), args)
authServiceAddressOverride, _ := common.ExtractValueFromArgumentList(fmt.Sprintf("--%v", consts.AuthServiceAddressOverrideParameter), args)
authServicePortOverride, _ := common.ExtractValueFromArgumentList(fmt.Sprintf("--%v", consts.AuthServicePortOverrideParameter), args)

authEP, err := foreigncluster.GetHomeAuthURL(ctx, clientSet, clientSet,
authServiceAddressOverride, authServicePortOverride, liqoNamespace)
Expand Down

0 comments on commit 6c48cac

Please sign in to comment.