TUN-7463: Add default ingress rule if no ingress rules are provided when updating the configuration
This commit is contained in:
		
							parent
							
								
									72737a200c
								
							
						
					
					
						commit
						f4ce7c6761
					
				|  | @ -399,7 +399,7 @@ func StartServer( | |||
| 		} | ||||
| 	} | ||||
| 
 | ||||
| 	localRules := []ingress.Rule{} | ||||
| 	internalRules := []ingress.Rule{} | ||||
| 	if features.Contains(features.FeatureManagementLogs) { | ||||
| 		serviceIP := c.String("service-op-ip") | ||||
| 		if edgeAddrs, err := edgediscovery.ResolveEdge(log, tunnelConfig.Region, tunnelConfig.EdgeIPVersion); err == nil { | ||||
|  | @ -416,9 +416,9 @@ func StartServer( | |||
| 			logger.ManagementLogger.Log, | ||||
| 			logger.ManagementLogger, | ||||
| 		) | ||||
| 		localRules = []ingress.Rule{ingress.NewManagementRule(mgmt)} | ||||
| 		internalRules = []ingress.Rule{ingress.NewManagementRule(mgmt)} | ||||
| 	} | ||||
| 	orchestrator, err := orchestration.NewOrchestrator(ctx, orchestratorConfig, tunnelConfig.Tags, localRules, tunnelConfig.Log) | ||||
| 	orchestrator, err := orchestration.NewOrchestrator(ctx, orchestratorConfig, tunnelConfig.Tags, internalRules, tunnelConfig.Log) | ||||
| 	if err != nil { | ||||
| 		return err | ||||
| 	} | ||||
|  |  | |||
|  | @ -44,7 +44,7 @@ func (ing Ingress) FindMatchingRule(hostname, path string) (*Rule, int) { | |||
| 	if err == nil { | ||||
| 		hostname = host | ||||
| 	} | ||||
| 	for i, rule := range ing.LocalRules { | ||||
| 	for i, rule := range ing.InternalRules { | ||||
| 		if rule.Matches(hostname, path) { | ||||
| 			// Local rule matches return a negative rule index to distiguish local rules from user-defined rules in logs
 | ||||
| 			// Full range would be [-1 .. )
 | ||||
|  | @ -77,7 +77,7 @@ func matchHost(ruleHost, reqHost string) bool { | |||
| // Ingress maps eyeball requests to origins.
 | ||||
| type Ingress struct { | ||||
| 	// Set of ingress rules that are not added to remote config, e.g. management
 | ||||
| 	LocalRules []Rule | ||||
| 	InternalRules []Rule | ||||
| 	// Rules that are provided by the user from remote or local configuration
 | ||||
| 	Rules    []Rule              `json:"ingress"` | ||||
| 	Defaults OriginRequestConfig `json:"originRequest"` | ||||
|  | @ -110,16 +110,18 @@ func ParseIngressFromConfigAndCLI(conf *config.Configuration, c *cli.Context, lo | |||
| 	//   --bastion for ssh bastion service
 | ||||
| 	ingressRules, err = parseCLIIngress(c, false) | ||||
| 	if errors.Is(err, ErrNoIngressRulesCLI) { | ||||
| 		// Only log a warning if the tunnel is not a remotely managed tunnel and the config
 | ||||
| 		// will be loaded after connecting.
 | ||||
| 		// If no token is provided, the probability of NOT being a remotely managed tunnel is higher.
 | ||||
| 		// So, we should warn the user that no ingress rules were found, because remote configuration will most likely not exist.
 | ||||
| 		if !c.IsSet("token") { | ||||
| 			log.Warn().Msgf(ErrNoIngressRulesCLI.Error()) | ||||
| 		} | ||||
| 		return newDefaultOrigin(c, log), nil | ||||
| 	} | ||||
| 
 | ||||
| 	if err != nil { | ||||
| 		return Ingress{}, err | ||||
| 	} | ||||
| 
 | ||||
| 	return ingressRules, nil | ||||
| } | ||||
| 
 | ||||
|  | @ -148,14 +150,10 @@ func parseCLIIngress(c *cli.Context, allowURLFromArgs bool) (Ingress, error) { | |||
| // newDefaultOrigin always returns a 503 response code to help indicate that there are no ingress
 | ||||
| // rules setup, but the tunnel is reachable.
 | ||||
| func newDefaultOrigin(c *cli.Context, log *zerolog.Logger) Ingress { | ||||
| 	noRulesService := newDefaultStatusCode(log) | ||||
| 	defaultRule := GetDefaultIngressRules(log) | ||||
| 	defaults := originRequestFromSingleRule(c) | ||||
| 	ingress := Ingress{ | ||||
| 		Rules: []Rule{ | ||||
| 			{ | ||||
| 				Service: &noRulesService, | ||||
| 			}, | ||||
| 		}, | ||||
| 		Rules:    defaultRule, | ||||
| 		Defaults: defaults, | ||||
| 	} | ||||
| 	return ingress | ||||
|  | @ -219,6 +217,17 @@ func (ing Ingress) CatchAll() *Rule { | |||
| 	return &ing.Rules[len(ing.Rules)-1] | ||||
| } | ||||
| 
 | ||||
| // Gets the default ingress rule that will be return 503 status
 | ||||
| // code for all incoming requests.
 | ||||
| func GetDefaultIngressRules(log *zerolog.Logger) []Rule { | ||||
| 	noRulesService := newDefaultStatusCode(log) | ||||
| 	return []Rule{ | ||||
| 		{ | ||||
| 			Service: &noRulesService, | ||||
| 		}, | ||||
| 	} | ||||
| } | ||||
| 
 | ||||
| func validateAccessConfiguration(cfg *config.AccessConfig) error { | ||||
| 	if !cfg.Required { | ||||
| 		return nil | ||||
|  |  | |||
|  | @ -28,8 +28,8 @@ type Orchestrator struct { | |||
| 	lock sync.RWMutex | ||||
| 	// Underlying value is proxy.Proxy, can be read without the lock, but still needs the lock to update
 | ||||
| 	proxy atomic.Value | ||||
| 	// Set of local ingress rules defined at cloudflared startup (separate from user-defined ingress rules)
 | ||||
| 	localRules         []ingress.Rule | ||||
| 	// Set of internal ingress rules defined at cloudflared startup (separate from user-defined ingress rules)
 | ||||
| 	internalRules      []ingress.Rule | ||||
| 	warpRoutingEnabled atomic.Bool | ||||
| 	config             *Config | ||||
| 	tags               []tunnelpogs.Tag | ||||
|  | @ -41,13 +41,13 @@ type Orchestrator struct { | |||
| 	proxyShutdownC chan<- struct{} | ||||
| } | ||||
| 
 | ||||
| func NewOrchestrator(ctx context.Context, config *Config, tags []tunnelpogs.Tag, localRules []ingress.Rule, log *zerolog.Logger) (*Orchestrator, error) { | ||||
| func NewOrchestrator(ctx context.Context, config *Config, tags []tunnelpogs.Tag, internalRules []ingress.Rule, log *zerolog.Logger) (*Orchestrator, error) { | ||||
| 	o := &Orchestrator{ | ||||
| 		// Lowest possible version, any remote configuration will have version higher than this
 | ||||
| 		// Starting at -1 allows a configuration migration (local to remote) to override the current configuration as it
 | ||||
| 		// will start at version 0.
 | ||||
| 		currentVersion: -1, | ||||
| 		localRules:     localRules, | ||||
| 		internalRules:  internalRules, | ||||
| 		config:         config, | ||||
| 		tags:           tags, | ||||
| 		log:            log, | ||||
|  | @ -116,8 +116,13 @@ func (o *Orchestrator) updateIngress(ingressRules ingress.Ingress, warpRouting i | |||
| 	default: | ||||
| 	} | ||||
| 
 | ||||
| 	// Assign the local ingress rules to the parsed ingress
 | ||||
| 	ingressRules.LocalRules = o.localRules | ||||
| 	// Assign the internal ingress rules to the parsed ingress
 | ||||
| 	ingressRules.InternalRules = o.internalRules | ||||
| 
 | ||||
| 	// Check if ingress rules are empty, and add the default route if so.
 | ||||
| 	if ingressRules.IsEmpty() { | ||||
| 		ingressRules.Rules = ingress.GetDefaultIngressRules(o.log) | ||||
| 	} | ||||
| 
 | ||||
| 	// Start new proxy before closing the ones from last version.
 | ||||
| 	// The upside is we don't need to restart proxy from last version, which can fail
 | ||||
|  |  | |||
|  | @ -91,15 +91,15 @@ func TestUpdateConfiguration(t *testing.T) { | |||
|         "enabled": true, | ||||
|         "connectTimeout": 10 | ||||
|     } | ||||
| }	 | ||||
| } | ||||
| `) | ||||
| 
 | ||||
| 	updateWithValidation(t, orchestrator, 2, configJSONV2) | ||||
| 	configV2 := orchestrator.config | ||||
| 	// Validate local ingress rules
 | ||||
| 	require.Equal(t, "management.argotunnel.com", configV2.Ingress.LocalRules[0].Hostname) | ||||
| 	require.True(t, configV2.Ingress.LocalRules[0].Matches("management.argotunnel.com", "/ping")) | ||||
| 	require.Equal(t, "management", configV2.Ingress.LocalRules[0].Service.String()) | ||||
| 	// Validate internal ingress rules
 | ||||
| 	require.Equal(t, "management.argotunnel.com", configV2.Ingress.InternalRules[0].Hostname) | ||||
| 	require.True(t, configV2.Ingress.InternalRules[0].Matches("management.argotunnel.com", "/ping")) | ||||
| 	require.Equal(t, "management", configV2.Ingress.InternalRules[0].Service.String()) | ||||
| 	// Validate ingress rule 0
 | ||||
| 	require.Equal(t, "jira.tunnel.org", configV2.Ingress.Rules[0].Hostname) | ||||
| 	require.True(t, configV2.Ingress.Rules[0].Matches("jira.tunnel.org", "/login")) | ||||
|  | @ -145,7 +145,7 @@ func TestUpdateConfiguration(t *testing.T) { | |||
| { | ||||
| 	"originRequest": | ||||
| } | ||||
| 	 | ||||
| 
 | ||||
| `) | ||||
| 
 | ||||
| 	resp = orchestrator.UpdateConfig(3, invalidJSON) | ||||
|  | @ -165,7 +165,7 @@ func TestUpdateConfiguration(t *testing.T) { | |||
|     "warp-routing": { | ||||
|         "enabled": false | ||||
|     } | ||||
| }	 | ||||
| } | ||||
| `) | ||||
| 	updateWithValidation(t, orchestrator, 10, configJSONV10) | ||||
| 	configV10 := orchestrator.config | ||||
|  | @ -204,12 +204,34 @@ func TestUpdateConfiguration_FromMigration(t *testing.T) { | |||
|     "warp-routing": { | ||||
|         "enabled": true | ||||
|     } | ||||
| }	 | ||||
| } | ||||
| `) | ||||
| 	updateWithValidation(t, orchestrator, 0, configJSONV2) | ||||
| 	require.Len(t, orchestrator.config.Ingress.Rules, 1) | ||||
| } | ||||
| 
 | ||||
| // Validates that the default ingress rule will be set if there is no rule provided from the remote.
 | ||||
| func TestUpdateConfiguration_WithoutIngressRule(t *testing.T) { | ||||
| 	initConfig := &Config{ | ||||
| 		Ingress: &ingress.Ingress{}, | ||||
| 	} | ||||
| 	orchestrator, err := NewOrchestrator(context.Background(), initConfig, testTags, []ingress.Rule{}, &testLogger) | ||||
| 	require.NoError(t, err) | ||||
| 	initOriginProxy, err := orchestrator.GetOriginProxy() | ||||
| 	require.NoError(t, err) | ||||
| 	require.Implements(t, (*connection.OriginProxy)(nil), initOriginProxy) | ||||
| 
 | ||||
| 	// We need to create an empty RemoteConfigJSON because that will get unmarshalled to a RemoteConfig
 | ||||
| 	emptyConfig := &ingress.RemoteConfigJSON{} | ||||
| 	configBytes, err := json.Marshal(emptyConfig) | ||||
| 	if err != nil { | ||||
| 		require.FailNow(t, "The RemoteConfigJSON shouldn't fail while being marshalled") | ||||
| 	} | ||||
| 
 | ||||
| 	updateWithValidation(t, orchestrator, 0, configBytes) | ||||
| 	require.Len(t, orchestrator.config.Ingress.Rules, 1) | ||||
| } | ||||
| 
 | ||||
| // TestConcurrentUpdateAndRead makes sure orchestrator can receive updates and return origin proxy concurrently
 | ||||
| func TestConcurrentUpdateAndRead(t *testing.T) { | ||||
| 	const ( | ||||
|  |  | |||
		Loading…
	
		Reference in New Issue