Skip to content

Commit

Permalink
Make admission plugin config a pointer
Browse files Browse the repository at this point in the history
  • Loading branch information
liggitt committed Jan 13, 2018
1 parent 516aea9 commit 73c3201
Show file tree
Hide file tree
Showing 28 changed files with 104 additions and 70 deletions.
2 changes: 1 addition & 1 deletion pkg/build/admission/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import (
)

// ReadPluginConfig will read a plugin configuration object from a reader stream
func ReadPluginConfig(pluginConfig map[string]configapi.AdmissionPluginConfig, name string, config runtime.Object) error {
func ReadPluginConfig(pluginConfig map[string]*configapi.AdmissionPluginConfig, name string, config runtime.Object) error {

configFilePath, err := pluginconfig.GetPluginConfigFile(pluginConfig, name, "")
if err != nil || len(configFilePath) == 0 {
Expand Down
6 changes: 3 additions & 3 deletions pkg/build/admission/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ func TestReadPluginConfig(t *testing.T) {
Item1: "hello",
Item2: []string{"foo", "bar"},
}
pluginCfg := map[string]configapi.AdmissionPluginConfig{"testconfig": {Location: "", Configuration: expected}}
pluginCfg := map[string]*configapi.AdmissionPluginConfig{"testconfig": {Location: "", Configuration: expected}}
// The config should match the expected config object
err := ReadPluginConfig(pluginCfg, "testconfig", config)
if err != nil {
Expand All @@ -34,15 +34,15 @@ func TestReadPluginConfig(t *testing.T) {
}

// Passing a nil cfg, should not get an error
pluginCfg = map[string]configapi.AdmissionPluginConfig{}
pluginCfg = map[string]*configapi.AdmissionPluginConfig{}
err = ReadPluginConfig(pluginCfg, "testconfig", &testtypes.TestConfig{})
if err != nil {
t.Fatalf("unexpected: %v", err)
}

// Passing the wrong type of destination object should result in an error
config2 := &testtypes.OtherTestConfig2{}
pluginCfg = map[string]configapi.AdmissionPluginConfig{"testconfig": {Location: "", Configuration: expected}}
pluginCfg = map[string]*configapi.AdmissionPluginConfig{"testconfig": {Location: "", Configuration: expected}}
err = ReadPluginConfig(pluginCfg, "testconfig", config2)
if err == nil {
t.Fatalf("expected error")
Expand Down
2 changes: 1 addition & 1 deletion pkg/build/controller/build/defaults/defaults.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ type BuildDefaults struct {
}

// NewBuildDefaults creates a new BuildDefaults that will apply the defaults specified in the plugin config
func NewBuildDefaults(pluginConfig map[string]configapi.AdmissionPluginConfig) (BuildDefaults, error) {
func NewBuildDefaults(pluginConfig map[string]*configapi.AdmissionPluginConfig) (BuildDefaults, error) {
config := &defaultsapi.BuildDefaultsConfig{}
err := buildadmission.ReadPluginConfig(pluginConfig, defaultsapi.BuildDefaultsPlugin, config)
if err != nil {
Expand Down
2 changes: 1 addition & 1 deletion pkg/build/controller/build/overrides/overrides.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ type BuildOverrides struct {
}

// NewBuildOverrides creates a new BuildOverrides that will apply the overrides specified in the plugin config
func NewBuildOverrides(pluginConfig map[string]configapi.AdmissionPluginConfig) (BuildOverrides, error) {
func NewBuildOverrides(pluginConfig map[string]*configapi.AdmissionPluginConfig) (BuildOverrides, error) {
config := &overridesapi.BuildOverridesConfig{}
err := buildadmission.ReadPluginConfig(pluginConfig, overridesapi.BuildOverridesPlugin, config)
if err != nil {
Expand Down
23 changes: 19 additions & 4 deletions pkg/cmd/server/api/serialization_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,7 @@ func fuzzInternalObject(t *testing.T, forVersion schema.GroupVersion, item runti

// test an admission plugin nested for round tripping
if c.RandBool() {
obj.AdmissionConfig.PluginConfig = map[string]configapi.AdmissionPluginConfig{
obj.AdmissionConfig.PluginConfig = map[string]*configapi.AdmissionPluginConfig{
"abc": {
Location: "test",
Configuration: &configapi.LDAPSyncConfig{
Expand All @@ -178,9 +178,24 @@ func fuzzInternalObject(t *testing.T, forVersion schema.GroupVersion, item runti
},
}
}

// ensure there are no nil plugin config objects
for pluginName := range obj.AdmissionConfig.PluginConfig {
if obj.AdmissionConfig.PluginConfig[pluginName] == nil {
obj.AdmissionConfig.PluginConfig[pluginName] = &configapi.AdmissionPluginConfig{}
}
}
if obj.KubernetesMasterConfig != nil {
for pluginName := range obj.KubernetesMasterConfig.AdmissionConfig.PluginConfig {
if obj.KubernetesMasterConfig.AdmissionConfig.PluginConfig[pluginName] == nil {
obj.KubernetesMasterConfig.AdmissionConfig.PluginConfig[pluginName] = &configapi.AdmissionPluginConfig{}
}
}
}

// test a Kubernetes admission plugin nested for round tripping
if obj.KubernetesMasterConfig != nil && c.RandBool() {
obj.KubernetesMasterConfig.AdmissionConfig.PluginConfig = map[string]configapi.AdmissionPluginConfig{
obj.KubernetesMasterConfig.AdmissionConfig.PluginConfig = map[string]*configapi.AdmissionPluginConfig{
"abc": {
Location: "test",
Configuration: &configapi.LDAPSyncConfig{
Expand Down Expand Up @@ -502,7 +517,7 @@ func TestSpecificRoundTrips(t *testing.T) {
{
in: &configapi.MasterConfig{
AdmissionConfig: configapi.AdmissionConfig{
PluginConfig: map[string]configapi.AdmissionPluginConfig{
PluginConfig: map[string]*configapi.AdmissionPluginConfig{
"test1": {Configuration: &configapi.LDAPSyncConfig{BindDN: "first"}},
"test2": {Configuration: &runtime.Unknown{Raw: []byte(`{"kind":"LDAPSyncConfig","apiVersion":"v1","bindDN":"second"}`)}},
"test3": {Configuration: &runtime.Unknown{Raw: []byte(`{"kind":"Unknown","apiVersion":"some/version"}`)}},
Expand All @@ -514,7 +529,7 @@ func TestSpecificRoundTrips(t *testing.T) {
out: &configapiv1.MasterConfig{
TypeMeta: metav1.TypeMeta{Kind: "MasterConfig", APIVersion: "v1"},
AdmissionConfig: configapiv1.AdmissionConfig{
PluginConfig: map[string]configapiv1.AdmissionPluginConfig{
PluginConfig: map[string]*configapiv1.AdmissionPluginConfig{
"test1": {Configuration: runtime.RawExtension{
Object: &configapiv1.LDAPSyncConfig{BindDN: "first"},
Raw: []byte(`{"kind":"LDAPSyncConfig","apiVersion":"v1","url":"","bindDN":"first","bindPassword":"","insecure":false,"ca":"","groupUIDNameMapping":null}`),
Expand Down
2 changes: 1 addition & 1 deletion pkg/cmd/server/api/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -1523,7 +1523,7 @@ type AdmissionPluginConfig struct {

type AdmissionConfig struct {
// PluginConfig allows specifying a configuration file per admission control plugin
PluginConfig map[string]AdmissionPluginConfig
PluginConfig map[string]*AdmissionPluginConfig

// PluginOrderOverride is a list of admission control plugin names that will be installed
// on the master. Order is significant. If empty, a default list of plugins is used.
Expand Down
13 changes: 13 additions & 0 deletions pkg/cmd/server/api/v1/conversions.go
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,13 @@ func SetDefaults_MasterConfig(obj *MasterConfig) {
// The final value of OAuthConfig.MasterCA should never be nil
obj.OAuthConfig.MasterCA = &s
}

// Ensure no nil plugin config stanzas
for pluginName := range obj.AdmissionConfig.PluginConfig {
if obj.AdmissionConfig.PluginConfig[pluginName] == nil {
obj.AdmissionConfig.PluginConfig[pluginName] = &AdmissionPluginConfig{}
}
}
}

func SetDefaults_KubernetesMasterConfig(obj *KubernetesMasterConfig) {
Expand All @@ -147,6 +154,12 @@ func SetDefaults_KubernetesMasterConfig(obj *KubernetesMasterConfig) {
if len(obj.PodEvictionTimeout) == 0 {
obj.PodEvictionTimeout = "5m"
}
// Ensure no nil plugin config stanzas
for pluginName := range obj.AdmissionConfig.PluginConfig {
if obj.AdmissionConfig.PluginConfig[pluginName] == nil {
obj.AdmissionConfig.PluginConfig[pluginName] = &AdmissionPluginConfig{}
}
}
}
func SetDefaults_NodeConfig(obj *NodeConfig) {
if obj.MasterClientConnectionOverrides == nil {
Expand Down
2 changes: 1 addition & 1 deletion pkg/cmd/server/api/v1/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -1443,7 +1443,7 @@ type AdmissionPluginConfig struct {
// AdmissionConfig holds the necessary configuration options for admission
type AdmissionConfig struct {
// PluginConfig allows specifying a configuration file per admission control plugin
PluginConfig map[string]AdmissionPluginConfig `json:"pluginConfig"`
PluginConfig map[string]*AdmissionPluginConfig `json:"pluginConfig"`

// PluginOrderOverride is a list of admission control plugin names that will be installed
// on the master. Order is significant. If empty, a default list of plugins is used.
Expand Down
4 changes: 2 additions & 2 deletions pkg/cmd/server/api/v1/types_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,7 @@ func TestMasterConfig(t *testing.T) {
},
KubernetesMasterConfig: &internal.KubernetesMasterConfig{
AdmissionConfig: internal.AdmissionConfig{
PluginConfig: map[string]internal.AdmissionPluginConfig{ // test config as an embedded object
PluginConfig: map[string]*internal.AdmissionPluginConfig{ // test config as an embedded object
"plugin": {
Configuration: &testtypes.AdmissionPluginTestConfig{},
},
Expand Down Expand Up @@ -229,7 +229,7 @@ func TestMasterConfig(t *testing.T) {
},
DNSConfig: &internal.DNSConfig{},
AdmissionConfig: internal.AdmissionConfig{
PluginConfig: map[string]internal.AdmissionPluginConfig{ // test config as an embedded object
PluginConfig: map[string]*internal.AdmissionPluginConfig{ // test config as an embedded object
"plugin": {
Configuration: &testtypes.AdmissionPluginTestConfig{},
},
Expand Down
11 changes: 7 additions & 4 deletions pkg/cmd/server/api/v1/zz_generated.deepcopy.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,11 +42,14 @@ func (in *AdmissionConfig) DeepCopyInto(out *AdmissionConfig) {
*out = *in
if in.PluginConfig != nil {
in, out := &in.PluginConfig, &out.PluginConfig
*out = make(map[string]AdmissionPluginConfig, len(*in))
*out = make(map[string]*AdmissionPluginConfig, len(*in))
for key, val := range *in {
newVal := new(AdmissionPluginConfig)
val.DeepCopyInto(newVal)
(*out)[key] = *newVal
if val == nil {
(*out)[key] = nil
} else {
(*out)[key] = new(AdmissionPluginConfig)
val.DeepCopyInto((*out)[key])
}
}
}
if in.PluginOrderOverride != nil {
Expand Down
2 changes: 1 addition & 1 deletion pkg/cmd/server/api/validation/master.go
Original file line number Diff line number Diff line change
Expand Up @@ -774,7 +774,7 @@ func deprecatedAdmissionPluginNames() sets.String {
return sets.NewString("openshift.io/OriginResourceQuota")
}

func ValidateAdmissionPluginConfig(pluginConfig map[string]api.AdmissionPluginConfig, fieldPath *field.Path) ValidationResults {
func ValidateAdmissionPluginConfig(pluginConfig map[string]*api.AdmissionPluginConfig, fieldPath *field.Path) ValidationResults {
validationResults := ValidationResults{}

deprecatedPlugins := deprecatedAdmissionPluginNames()
Expand Down
38 changes: 19 additions & 19 deletions pkg/cmd/server/api/validation/master_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -215,34 +215,34 @@ func TestValidateAdmissionPluginConfig(t *testing.T) {
bothEmpty := configapi.AdmissionPluginConfig{}

tests := []struct {
config map[string]configapi.AdmissionPluginConfig
config map[string]*configapi.AdmissionPluginConfig
expectError bool
warningFields []string
}{
{
config: map[string]configapi.AdmissionPluginConfig{
"one": locationOnly,
"two": configOnly,
config: map[string]*configapi.AdmissionPluginConfig{
"one": &locationOnly,
"two": &configOnly,
},
},
{
config: map[string]configapi.AdmissionPluginConfig{
"one": locationOnly,
"two": locationAndConfig,
config: map[string]*configapi.AdmissionPluginConfig{
"one": &locationOnly,
"two": &locationAndConfig,
},
expectError: true,
},
{
config: map[string]configapi.AdmissionPluginConfig{
"one": configOnly,
"two": bothEmpty,
config: map[string]*configapi.AdmissionPluginConfig{
"one": &configOnly,
"two": &bothEmpty,
},
expectError: true,
},
{
config: map[string]configapi.AdmissionPluginConfig{
"openshift.io/OriginResourceQuota": configOnly,
"two": configOnly,
config: map[string]*configapi.AdmissionPluginConfig{
"openshift.io/OriginResourceQuota": &configOnly,
"two": &configOnly,
},
warningFields: []string{"[openshift.io/OriginResourceQuota]"},
expectError: false,
Expand Down Expand Up @@ -325,7 +325,7 @@ func TestValidateAdmissionPluginConfigConflicts(t *testing.T) {
name: "specified, non-conflicting plugin configs 01",
options: configapi.MasterConfig{
AdmissionConfig: configapi.AdmissionConfig{
PluginConfig: map[string]configapi.AdmissionPluginConfig{
PluginConfig: map[string]*configapi.AdmissionPluginConfig{
"foo": {
Location: "bar",
},
Expand All @@ -338,7 +338,7 @@ func TestValidateAdmissionPluginConfigConflicts(t *testing.T) {
options: configapi.MasterConfig{
KubernetesMasterConfig: &configapi.KubernetesMasterConfig{
AdmissionConfig: configapi.AdmissionConfig{
PluginConfig: map[string]configapi.AdmissionPluginConfig{
PluginConfig: map[string]*configapi.AdmissionPluginConfig{
"foo": {
Location: "bar",
},
Expand All @@ -349,7 +349,7 @@ func TestValidateAdmissionPluginConfigConflicts(t *testing.T) {
},
},
AdmissionConfig: configapi.AdmissionConfig{
PluginConfig: map[string]configapi.AdmissionPluginConfig{
PluginConfig: map[string]*configapi.AdmissionPluginConfig{
"foo": {
Location: "bar",
},
Expand All @@ -362,7 +362,7 @@ func TestValidateAdmissionPluginConfigConflicts(t *testing.T) {
options: configapi.MasterConfig{
KubernetesMasterConfig: &configapi.KubernetesMasterConfig{
AdmissionConfig: configapi.AdmissionConfig{
PluginConfig: map[string]configapi.AdmissionPluginConfig{
PluginConfig: map[string]*configapi.AdmissionPluginConfig{
"foo": {
Location: "bar",
},
Expand All @@ -379,15 +379,15 @@ func TestValidateAdmissionPluginConfigConflicts(t *testing.T) {
options: configapi.MasterConfig{
KubernetesMasterConfig: &configapi.KubernetesMasterConfig{
AdmissionConfig: configapi.AdmissionConfig{
PluginConfig: map[string]configapi.AdmissionPluginConfig{
PluginConfig: map[string]*configapi.AdmissionPluginConfig{
"foo": {
Location: "different",
},
},
},
},
AdmissionConfig: configapi.AdmissionConfig{
PluginConfig: map[string]configapi.AdmissionPluginConfig{
PluginConfig: map[string]*configapi.AdmissionPluginConfig{
"foo": {
Location: "bar",
},
Expand Down
11 changes: 7 additions & 4 deletions pkg/cmd/server/api/zz_generated.deepcopy.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,11 +42,14 @@ func (in *AdmissionConfig) DeepCopyInto(out *AdmissionConfig) {
*out = *in
if in.PluginConfig != nil {
in, out := &in.PluginConfig, &out.PluginConfig
*out = make(map[string]AdmissionPluginConfig, len(*in))
*out = make(map[string]*AdmissionPluginConfig, len(*in))
for key, val := range *in {
newVal := new(AdmissionPluginConfig)
val.DeepCopyInto(newVal)
(*out)[key] = *newVal
if val == nil {
(*out)[key] = nil
} else {
(*out)[key] = new(AdmissionPluginConfig)
val.DeepCopyInto((*out)[key])
}
}
}
if in.PluginOrderOverride != nil {
Expand Down
2 changes: 1 addition & 1 deletion pkg/cmd/server/origin/admission/chain_builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@ func NewAdmissionChains(
} else {
pluginConfig := map[string]configapi.AdmissionPluginConfig{}
for pluginName, config := range options.AdmissionConfig.PluginConfig {
pluginConfig[pluginName] = config
pluginConfig[pluginName] = *config
}
upstreamAdmissionConfig, err := configapilatest.ConvertOpenshiftAdmissionConfigToKubeAdmissionConfig(pluginConfig)
if err != nil {
Expand Down
2 changes: 1 addition & 1 deletion pkg/cmd/server/origin/admission/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,7 @@ func TestSeparateAdmissionChainDetection(t *testing.T) {
options: configapi.MasterConfig{
KubernetesMasterConfig: &configapi.KubernetesMasterConfig{},
AdmissionConfig: configapi.AdmissionConfig{
PluginConfig: map[string]configapi.AdmissionPluginConfig{
PluginConfig: map[string]*configapi.AdmissionPluginConfig{
"foo": {
Location: "bar",
},
Expand Down
2 changes: 1 addition & 1 deletion pkg/cmd/server/origin/controller/build.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import (
type BuildControllerConfig struct {
DockerImage string
S2IImage string
AdmissionPluginConfig map[string]configapi.AdmissionPluginConfig
AdmissionPluginConfig map[string]*configapi.AdmissionPluginConfig

Codec runtime.Codec
}
Expand Down
4 changes: 2 additions & 2 deletions pkg/cmd/util/pluginconfig/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,11 +47,11 @@ func GetPluginConfig(cfg configapi.AdmissionPluginConfig) (string, error) {

// GetPluginConfigFile translates from the master plugin config to a file name containing
// a particular plugin's config (the file may be a temp file if config is embedded)
func GetPluginConfigFile(pluginConfig map[string]configapi.AdmissionPluginConfig, pluginName string, defaultConfigFilePath string) (string, error) {
func GetPluginConfigFile(pluginConfig map[string]*configapi.AdmissionPluginConfig, pluginName string, defaultConfigFilePath string) (string, error) {
// Check whether a config is specified for this plugin. If not, default to the
// global plugin config file (if any).
if cfg, hasConfig := pluginConfig[pluginName]; hasConfig {
configFilePath, err := GetPluginConfig(cfg)
configFilePath, err := GetPluginConfig(*cfg)
if err != nil {
return "", err
}
Expand Down
10 changes: 5 additions & 5 deletions pkg/oc/bootstrap/docker/openshift/helper.go
Original file line number Diff line number Diff line change
Expand Up @@ -703,9 +703,9 @@ func (h *Helper) updateConfig(configDir string, opt *StartOptions) error {

// turn on admission webhooks by default. They are no-ops until someone explicitly tries to configure one
if cfg.AdmissionConfig.PluginConfig == nil {
cfg.AdmissionConfig.PluginConfig = map[string]configapi.AdmissionPluginConfig{}
cfg.AdmissionConfig.PluginConfig = map[string]*configapi.AdmissionPluginConfig{}
}
cfg.AdmissionConfig.PluginConfig["GenericAdmissionWebhook"] = configapi.AdmissionPluginConfig{
cfg.AdmissionConfig.PluginConfig["GenericAdmissionWebhook"] = &configapi.AdmissionPluginConfig{
Configuration: &configapi.DefaultAdmissionConfig{},
}

Expand Down Expand Up @@ -733,7 +733,7 @@ func (h *Helper) updateConfig(configDir string, opt *StartOptions) error {
var buildDefaults *defaultsapi.BuildDefaultsConfig
buildDefaultsConfig, ok := cfg.AdmissionConfig.PluginConfig[defaultsapi.BuildDefaultsPlugin]
if !ok {
buildDefaultsConfig = configapi.AdmissionPluginConfig{}
buildDefaultsConfig = &configapi.AdmissionPluginConfig{}
}
if buildDefaultsConfig.Configuration != nil {
buildDefaults = buildDefaultsConfig.Configuration.(*defaultsapi.BuildDefaultsConfig)
Expand Down Expand Up @@ -837,10 +837,10 @@ func (h *Helper) updateConfig(configDir string, opt *StartOptions) error {
cfg.KubernetesMasterConfig.APIServerArguments["runtime-config"] = append(cfg.KubernetesMasterConfig.APIServerArguments["runtime-config"], "apis/settings.k8s.io/v1alpha1=true")

if cfg.AdmissionConfig.PluginConfig == nil {
cfg.AdmissionConfig.PluginConfig = map[string]configapi.AdmissionPluginConfig{}
cfg.AdmissionConfig.PluginConfig = map[string]*configapi.AdmissionPluginConfig{}
}

cfg.AdmissionConfig.PluginConfig["PodPreset"] = configapi.AdmissionPluginConfig{
cfg.AdmissionConfig.PluginConfig["PodPreset"] = &configapi.AdmissionPluginConfig{
Configuration: &configapi.DefaultAdmissionConfig{Disable: false},
}

Expand Down
Loading

0 comments on commit 73c3201

Please sign in to comment.