diff --git a/pkg/apis/batch/validation/validation.go b/pkg/apis/batch/validation/validation.go index 2efb83bb064..26c365b2f98 100644 --- a/pkg/apis/batch/validation/validation.go +++ b/pkg/apis/batch/validation/validation.go @@ -18,10 +18,12 @@ package validation import ( "fmt" + "regexp" "strings" "time" "github.com/robfig/cron/v3" + v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" unversionedvalidation "k8s.io/apimachinery/pkg/apis/meta/v1/validation" @@ -502,6 +504,16 @@ func validateScheduleFormat(schedule string, timeZone *string, fldPath *field.Pa return allErrs } +// https://data.iana.org/time-zones/theory.html#naming +// * A name must not be empty, or contain '//', or start or end with '/'. +// * Do not use the file name components '.' and '..'. +// * Within a file name component, use only ASCII letters, '.', '-' and '_'. +// * Do not use digits, as that might create an ambiguity with POSIX TZ strings. +// * A file name component must not exceed 14 characters or start with '-' +// +// 0-9 and + characters are tolerated to accommodate legacy compatibility names +var validTimeZoneCharacters = regexp.MustCompile(`^[A-Za-z\.\-_0-9+]{1,14}$`) + func validateTimeZone(timeZone *string, fldPath *field.Path) field.ErrorList { allErrs := field.ErrorList{} if timeZone == nil { @@ -513,6 +525,13 @@ func validateTimeZone(timeZone *string, fldPath *field.Path) field.ErrorList { return allErrs } + for _, part := range strings.Split(*timeZone, "/") { + if part == "." || part == ".." || strings.HasPrefix(part, "-") || !validTimeZoneCharacters.MatchString(part) { + allErrs = append(allErrs, field.Invalid(fldPath, timeZone, fmt.Sprintf("unknown time zone %s", *timeZone))) + return allErrs + } + } + if strings.EqualFold(*timeZone, "Local") { allErrs = append(allErrs, field.Invalid(fldPath, timeZone, "timeZone must be an explicit time zone as defined in https://www.iana.org/time-zones")) } diff --git a/pkg/apis/batch/validation/validation_test.go b/pkg/apis/batch/validation/validation_test.go index 8ac6af9593b..401950fc8a6 100644 --- a/pkg/apis/batch/validation/validation_test.go +++ b/pkg/apis/batch/validation/validation_test.go @@ -17,6 +17,8 @@ limitations under the License. package validation import ( + _ "time/tzdata" + "archive/zip" "fmt" "io" @@ -28,6 +30,7 @@ import ( "github.com/google/go-cmp/cmp" "github.com/google/go-cmp/cmp/cmpopts" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/validation/field" @@ -1652,6 +1655,23 @@ func TestValidateCronJob(t *testing.T) { }, }, }, + "spec.timeZone: Invalid value: \"Continent/Zone \": unknown time zone Continent/Zone ": { + ObjectMeta: metav1.ObjectMeta{ + Name: "mycronjob", + Namespace: metav1.NamespaceDefault, + UID: types.UID("1a2b3c"), + }, + Spec: batch.CronJobSpec{ + Schedule: "0 * * * *", + TimeZone: &timeZoneBadSuffix, + ConcurrencyPolicy: batch.AllowConcurrent, + JobTemplate: batch.JobTemplateSpec{ + Spec: batch.JobSpec{ + Template: validPodTemplateSpec, + }, + }, + }, + }, "spec.startingDeadlineSeconds:must be greater than or equal to 0": { ObjectMeta: metav1.ObjectMeta{ Name: "mycronjob", @@ -1883,28 +1903,6 @@ func TestValidateCronJob(t *testing.T) { }, } - // NOTE: The following test doesn't fail on Windows. - // When opening a file on Windows, syscall.Open is called, which will trim the whitespace suffix. - if runtime.GOOS != "windows" { - errorCases["spec.timeZone: Invalid value: \"Continent/Zone \": unknown time zone Continent/Zone "] = batch.CronJob{ - ObjectMeta: metav1.ObjectMeta{ - Name: "mycronjob", - Namespace: metav1.NamespaceDefault, - UID: types.UID("1a2b3c"), - }, - Spec: batch.CronJobSpec{ - Schedule: "0 * * * *", - TimeZone: &timeZoneBadSuffix, - ConcurrencyPolicy: batch.AllowConcurrent, - JobTemplate: batch.JobTemplateSpec{ - Spec: batch.JobSpec{ - Template: validPodTemplateSpec, - }, - }, - }, - } - } - for k, v := range errorCases { t.Run(k, func(t *testing.T) { errs := ValidateCronJobCreate(&v, corevalidation.PodValidationOptions{}) @@ -2185,3 +2183,610 @@ func TestValidateCronJobSpec(t *testing.T) { func completionModePtr(m batch.CompletionMode) *batch.CompletionMode { return &m } + +func TestTimeZones(t *testing.T) { + // all valid time zones as of go1.19 release on 2022-08-02 + data := []string{ + `Africa/Abidjan`, + `Africa/Accra`, + `Africa/Addis_Ababa`, + `Africa/Algiers`, + `Africa/Asmara`, + `Africa/Asmera`, + `Africa/Bamako`, + `Africa/Bangui`, + `Africa/Banjul`, + `Africa/Bissau`, + `Africa/Blantyre`, + `Africa/Brazzaville`, + `Africa/Bujumbura`, + `Africa/Cairo`, + `Africa/Casablanca`, + `Africa/Ceuta`, + `Africa/Conakry`, + `Africa/Dakar`, + `Africa/Dar_es_Salaam`, + `Africa/Djibouti`, + `Africa/Douala`, + `Africa/El_Aaiun`, + `Africa/Freetown`, + `Africa/Gaborone`, + `Africa/Harare`, + `Africa/Johannesburg`, + `Africa/Juba`, + `Africa/Kampala`, + `Africa/Khartoum`, + `Africa/Kigali`, + `Africa/Kinshasa`, + `Africa/Lagos`, + `Africa/Libreville`, + `Africa/Lome`, + `Africa/Luanda`, + `Africa/Lubumbashi`, + `Africa/Lusaka`, + `Africa/Malabo`, + `Africa/Maputo`, + `Africa/Maseru`, + `Africa/Mbabane`, + `Africa/Mogadishu`, + `Africa/Monrovia`, + `Africa/Nairobi`, + `Africa/Ndjamena`, + `Africa/Niamey`, + `Africa/Nouakchott`, + `Africa/Ouagadougou`, + `Africa/Porto-Novo`, + `Africa/Sao_Tome`, + `Africa/Timbuktu`, + `Africa/Tripoli`, + `Africa/Tunis`, + `Africa/Windhoek`, + `America/Adak`, + `America/Anchorage`, + `America/Anguilla`, + `America/Antigua`, + `America/Araguaina`, + `America/Argentina/Buenos_Aires`, + `America/Argentina/Catamarca`, + `America/Argentina/ComodRivadavia`, + `America/Argentina/Cordoba`, + `America/Argentina/Jujuy`, + `America/Argentina/La_Rioja`, + `America/Argentina/Mendoza`, + `America/Argentina/Rio_Gallegos`, + `America/Argentina/Salta`, + `America/Argentina/San_Juan`, + `America/Argentina/San_Luis`, + `America/Argentina/Tucuman`, + `America/Argentina/Ushuaia`, + `America/Aruba`, + `America/Asuncion`, + `America/Atikokan`, + `America/Atka`, + `America/Bahia`, + `America/Bahia_Banderas`, + `America/Barbados`, + `America/Belem`, + `America/Belize`, + `America/Blanc-Sablon`, + `America/Boa_Vista`, + `America/Bogota`, + `America/Boise`, + `America/Buenos_Aires`, + `America/Cambridge_Bay`, + `America/Campo_Grande`, + `America/Cancun`, + `America/Caracas`, + `America/Catamarca`, + `America/Cayenne`, + `America/Cayman`, + `America/Chicago`, + `America/Chihuahua`, + `America/Coral_Harbour`, + `America/Cordoba`, + `America/Costa_Rica`, + `America/Creston`, + `America/Cuiaba`, + `America/Curacao`, + `America/Danmarkshavn`, + `America/Dawson`, + `America/Dawson_Creek`, + `America/Denver`, + `America/Detroit`, + `America/Dominica`, + `America/Edmonton`, + `America/Eirunepe`, + `America/El_Salvador`, + `America/Ensenada`, + `America/Fort_Nelson`, + `America/Fort_Wayne`, + `America/Fortaleza`, + `America/Glace_Bay`, + `America/Godthab`, + `America/Goose_Bay`, + `America/Grand_Turk`, + `America/Grenada`, + `America/Guadeloupe`, + `America/Guatemala`, + `America/Guayaquil`, + `America/Guyana`, + `America/Halifax`, + `America/Havana`, + `America/Hermosillo`, + `America/Indiana/Indianapolis`, + `America/Indiana/Knox`, + `America/Indiana/Marengo`, + `America/Indiana/Petersburg`, + `America/Indiana/Tell_City`, + `America/Indiana/Vevay`, + `America/Indiana/Vincennes`, + `America/Indiana/Winamac`, + `America/Indianapolis`, + `America/Inuvik`, + `America/Iqaluit`, + `America/Jamaica`, + `America/Jujuy`, + `America/Juneau`, + `America/Kentucky/Louisville`, + `America/Kentucky/Monticello`, + `America/Knox_IN`, + `America/Kralendijk`, + `America/La_Paz`, + `America/Lima`, + `America/Los_Angeles`, + `America/Louisville`, + `America/Lower_Princes`, + `America/Maceio`, + `America/Managua`, + `America/Manaus`, + `America/Marigot`, + `America/Martinique`, + `America/Matamoros`, + `America/Mazatlan`, + `America/Mendoza`, + `America/Menominee`, + `America/Merida`, + `America/Metlakatla`, + `America/Mexico_City`, + `America/Miquelon`, + `America/Moncton`, + `America/Monterrey`, + `America/Montevideo`, + `America/Montreal`, + `America/Montserrat`, + `America/Nassau`, + `America/New_York`, + `America/Nipigon`, + `America/Nome`, + `America/Noronha`, + `America/North_Dakota/Beulah`, + `America/North_Dakota/Center`, + `America/North_Dakota/New_Salem`, + `America/Nuuk`, + `America/Ojinaga`, + `America/Panama`, + `America/Pangnirtung`, + `America/Paramaribo`, + `America/Phoenix`, + `America/Port-au-Prince`, + `America/Port_of_Spain`, + `America/Porto_Acre`, + `America/Porto_Velho`, + `America/Puerto_Rico`, + `America/Punta_Arenas`, + `America/Rainy_River`, + `America/Rankin_Inlet`, + `America/Recife`, + `America/Regina`, + `America/Resolute`, + `America/Rio_Branco`, + `America/Rosario`, + `America/Santa_Isabel`, + `America/Santarem`, + `America/Santiago`, + `America/Santo_Domingo`, + `America/Sao_Paulo`, + `America/Scoresbysund`, + `America/Shiprock`, + `America/Sitka`, + `America/St_Barthelemy`, + `America/St_Johns`, + `America/St_Kitts`, + `America/St_Lucia`, + `America/St_Thomas`, + `America/St_Vincent`, + `America/Swift_Current`, + `America/Tegucigalpa`, + `America/Thule`, + `America/Thunder_Bay`, + `America/Tijuana`, + `America/Toronto`, + `America/Tortola`, + `America/Vancouver`, + `America/Virgin`, + `America/Whitehorse`, + `America/Winnipeg`, + `America/Yakutat`, + `America/Yellowknife`, + `Antarctica/Casey`, + `Antarctica/Davis`, + `Antarctica/DumontDUrville`, + `Antarctica/Macquarie`, + `Antarctica/Mawson`, + `Antarctica/McMurdo`, + `Antarctica/Palmer`, + `Antarctica/Rothera`, + `Antarctica/South_Pole`, + `Antarctica/Syowa`, + `Antarctica/Troll`, + `Antarctica/Vostok`, + `Arctic/Longyearbyen`, + `Asia/Aden`, + `Asia/Almaty`, + `Asia/Amman`, + `Asia/Anadyr`, + `Asia/Aqtau`, + `Asia/Aqtobe`, + `Asia/Ashgabat`, + `Asia/Ashkhabad`, + `Asia/Atyrau`, + `Asia/Baghdad`, + `Asia/Bahrain`, + `Asia/Baku`, + `Asia/Bangkok`, + `Asia/Barnaul`, + `Asia/Beirut`, + `Asia/Bishkek`, + `Asia/Brunei`, + `Asia/Calcutta`, + `Asia/Chita`, + `Asia/Choibalsan`, + `Asia/Chongqing`, + `Asia/Chungking`, + `Asia/Colombo`, + `Asia/Dacca`, + `Asia/Damascus`, + `Asia/Dhaka`, + `Asia/Dili`, + `Asia/Dubai`, + `Asia/Dushanbe`, + `Asia/Famagusta`, + `Asia/Gaza`, + `Asia/Harbin`, + `Asia/Hebron`, + `Asia/Ho_Chi_Minh`, + `Asia/Hong_Kong`, + `Asia/Hovd`, + `Asia/Irkutsk`, + `Asia/Istanbul`, + `Asia/Jakarta`, + `Asia/Jayapura`, + `Asia/Jerusalem`, + `Asia/Kabul`, + `Asia/Kamchatka`, + `Asia/Karachi`, + `Asia/Kashgar`, + `Asia/Kathmandu`, + `Asia/Katmandu`, + `Asia/Khandyga`, + `Asia/Kolkata`, + `Asia/Krasnoyarsk`, + `Asia/Kuala_Lumpur`, + `Asia/Kuching`, + `Asia/Kuwait`, + `Asia/Macao`, + `Asia/Macau`, + `Asia/Magadan`, + `Asia/Makassar`, + `Asia/Manila`, + `Asia/Muscat`, + `Asia/Nicosia`, + `Asia/Novokuznetsk`, + `Asia/Novosibirsk`, + `Asia/Omsk`, + `Asia/Oral`, + `Asia/Phnom_Penh`, + `Asia/Pontianak`, + `Asia/Pyongyang`, + `Asia/Qatar`, + `Asia/Qostanay`, + `Asia/Qyzylorda`, + `Asia/Rangoon`, + `Asia/Riyadh`, + `Asia/Saigon`, + `Asia/Sakhalin`, + `Asia/Samarkand`, + `Asia/Seoul`, + `Asia/Shanghai`, + `Asia/Singapore`, + `Asia/Srednekolymsk`, + `Asia/Taipei`, + `Asia/Tashkent`, + `Asia/Tbilisi`, + `Asia/Tehran`, + `Asia/Tel_Aviv`, + `Asia/Thimbu`, + `Asia/Thimphu`, + `Asia/Tokyo`, + `Asia/Tomsk`, + `Asia/Ujung_Pandang`, + `Asia/Ulaanbaatar`, + `Asia/Ulan_Bator`, + `Asia/Urumqi`, + `Asia/Ust-Nera`, + `Asia/Vientiane`, + `Asia/Vladivostok`, + `Asia/Yakutsk`, + `Asia/Yangon`, + `Asia/Yekaterinburg`, + `Asia/Yerevan`, + `Atlantic/Azores`, + `Atlantic/Bermuda`, + `Atlantic/Canary`, + `Atlantic/Cape_Verde`, + `Atlantic/Faeroe`, + `Atlantic/Faroe`, + `Atlantic/Jan_Mayen`, + `Atlantic/Madeira`, + `Atlantic/Reykjavik`, + `Atlantic/South_Georgia`, + `Atlantic/St_Helena`, + `Atlantic/Stanley`, + `Australia/ACT`, + `Australia/Adelaide`, + `Australia/Brisbane`, + `Australia/Broken_Hill`, + `Australia/Canberra`, + `Australia/Currie`, + `Australia/Darwin`, + `Australia/Eucla`, + `Australia/Hobart`, + `Australia/LHI`, + `Australia/Lindeman`, + `Australia/Lord_Howe`, + `Australia/Melbourne`, + `Australia/North`, + `Australia/NSW`, + `Australia/Perth`, + `Australia/Queensland`, + `Australia/South`, + `Australia/Sydney`, + `Australia/Tasmania`, + `Australia/Victoria`, + `Australia/West`, + `Australia/Yancowinna`, + `Brazil/Acre`, + `Brazil/DeNoronha`, + `Brazil/East`, + `Brazil/West`, + `Canada/Atlantic`, + `Canada/Central`, + `Canada/Eastern`, + `Canada/Mountain`, + `Canada/Newfoundland`, + `Canada/Pacific`, + `Canada/Saskatchewan`, + `Canada/Yukon`, + `CET`, + `Chile/Continental`, + `Chile/EasterIsland`, + `CST6CDT`, + `Cuba`, + `EET`, + `Egypt`, + `Eire`, + `EST`, + `EST5EDT`, + `Etc/GMT`, + `Etc/GMT+0`, + `Etc/GMT+1`, + `Etc/GMT+10`, + `Etc/GMT+11`, + `Etc/GMT+12`, + `Etc/GMT+2`, + `Etc/GMT+3`, + `Etc/GMT+4`, + `Etc/GMT+5`, + `Etc/GMT+6`, + `Etc/GMT+7`, + `Etc/GMT+8`, + `Etc/GMT+9`, + `Etc/GMT-0`, + `Etc/GMT-1`, + `Etc/GMT-10`, + `Etc/GMT-11`, + `Etc/GMT-12`, + `Etc/GMT-13`, + `Etc/GMT-14`, + `Etc/GMT-2`, + `Etc/GMT-3`, + `Etc/GMT-4`, + `Etc/GMT-5`, + `Etc/GMT-6`, + `Etc/GMT-7`, + `Etc/GMT-8`, + `Etc/GMT-9`, + `Etc/GMT0`, + `Etc/Greenwich`, + `Etc/UCT`, + `Etc/Universal`, + `Etc/UTC`, + `Etc/Zulu`, + `Europe/Amsterdam`, + `Europe/Andorra`, + `Europe/Astrakhan`, + `Europe/Athens`, + `Europe/Belfast`, + `Europe/Belgrade`, + `Europe/Berlin`, + `Europe/Bratislava`, + `Europe/Brussels`, + `Europe/Bucharest`, + `Europe/Budapest`, + `Europe/Busingen`, + `Europe/Chisinau`, + `Europe/Copenhagen`, + `Europe/Dublin`, + `Europe/Gibraltar`, + `Europe/Guernsey`, + `Europe/Helsinki`, + `Europe/Isle_of_Man`, + `Europe/Istanbul`, + `Europe/Jersey`, + `Europe/Kaliningrad`, + `Europe/Kiev`, + `Europe/Kirov`, + `Europe/Lisbon`, + `Europe/Ljubljana`, + `Europe/London`, + `Europe/Luxembourg`, + `Europe/Madrid`, + `Europe/Malta`, + `Europe/Mariehamn`, + `Europe/Minsk`, + `Europe/Monaco`, + `Europe/Moscow`, + `Europe/Nicosia`, + `Europe/Oslo`, + `Europe/Paris`, + `Europe/Podgorica`, + `Europe/Prague`, + `Europe/Riga`, + `Europe/Rome`, + `Europe/Samara`, + `Europe/San_Marino`, + `Europe/Sarajevo`, + `Europe/Saratov`, + `Europe/Simferopol`, + `Europe/Skopje`, + `Europe/Sofia`, + `Europe/Stockholm`, + `Europe/Tallinn`, + `Europe/Tirane`, + `Europe/Tiraspol`, + `Europe/Ulyanovsk`, + `Europe/Uzhgorod`, + `Europe/Vaduz`, + `Europe/Vatican`, + `Europe/Vienna`, + `Europe/Vilnius`, + `Europe/Volgograd`, + `Europe/Warsaw`, + `Europe/Zagreb`, + `Europe/Zaporozhye`, + `Europe/Zurich`, + `Factory`, + `GB`, + `GB-Eire`, + `GMT`, + `GMT+0`, + `GMT-0`, + `GMT0`, + `Greenwich`, + `Hongkong`, + `HST`, + `Iceland`, + `Indian/Antananarivo`, + `Indian/Chagos`, + `Indian/Christmas`, + `Indian/Cocos`, + `Indian/Comoro`, + `Indian/Kerguelen`, + `Indian/Mahe`, + `Indian/Maldives`, + `Indian/Mauritius`, + `Indian/Mayotte`, + `Indian/Reunion`, + `Iran`, + `Israel`, + `Jamaica`, + `Japan`, + `Kwajalein`, + `Libya`, + `MET`, + `Mexico/BajaNorte`, + `Mexico/BajaSur`, + `Mexico/General`, + `MST`, + `MST7MDT`, + `Navajo`, + `NZ`, + `NZ-CHAT`, + `Pacific/Apia`, + `Pacific/Auckland`, + `Pacific/Bougainville`, + `Pacific/Chatham`, + `Pacific/Chuuk`, + `Pacific/Easter`, + `Pacific/Efate`, + `Pacific/Enderbury`, + `Pacific/Fakaofo`, + `Pacific/Fiji`, + `Pacific/Funafuti`, + `Pacific/Galapagos`, + `Pacific/Gambier`, + `Pacific/Guadalcanal`, + `Pacific/Guam`, + `Pacific/Honolulu`, + `Pacific/Johnston`, + `Pacific/Kanton`, + `Pacific/Kiritimati`, + `Pacific/Kosrae`, + `Pacific/Kwajalein`, + `Pacific/Majuro`, + `Pacific/Marquesas`, + `Pacific/Midway`, + `Pacific/Nauru`, + `Pacific/Niue`, + `Pacific/Norfolk`, + `Pacific/Noumea`, + `Pacific/Pago_Pago`, + `Pacific/Palau`, + `Pacific/Pitcairn`, + `Pacific/Pohnpei`, + `Pacific/Ponape`, + `Pacific/Port_Moresby`, + `Pacific/Rarotonga`, + `Pacific/Saipan`, + `Pacific/Samoa`, + `Pacific/Tahiti`, + `Pacific/Tarawa`, + `Pacific/Tongatapu`, + `Pacific/Truk`, + `Pacific/Wake`, + `Pacific/Wallis`, + `Pacific/Yap`, + `Poland`, + `Portugal`, + `PRC`, + `PST8PDT`, + `ROC`, + `ROK`, + `Singapore`, + `Turkey`, + `UCT`, + `Universal`, + `US/Alaska`, + `US/Aleutian`, + `US/Arizona`, + `US/Central`, + `US/East-Indiana`, + `US/Eastern`, + `US/Hawaii`, + `US/Indiana-Starke`, + `US/Michigan`, + `US/Mountain`, + `US/Pacific`, + `US/Samoa`, + `UTC`, + `W-SU`, + `WET`, + `Zulu`, + } + for _, tz := range data { + errs := validateTimeZone(&tz, nil) + if len(errs) > 0 { + t.Errorf("%s failed: %v", tz, errs) + } + } +}